From f8da068cc7a2884f34907f6c72c6394d1a309c7e Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Wed, 9 Aug 2023 00:54:47 -0700 Subject: [PATCH 1/7] [reggen,hw] Create index parameter for registers windows Signed-off-by: Robert Schilling --- hw/ip/flash_ctrl/rtl/flash_ctrl_reg_pkg.sv | 2 ++ hw/ip/hmac/rtl/hmac_reg_pkg.sv | 1 + hw/ip/kmac/rtl/kmac_reg_pkg.sv | 2 ++ hw/ip/otbn/rtl/otbn_reg_pkg.sv | 2 ++ hw/ip/otp_ctrl/rtl/otp_ctrl_reg_pkg.sv | 1 + hw/ip/rom_ctrl/rtl/rom_ctrl_reg_pkg.sv | 1 + hw/ip/rv_core_ibex/rtl/rv_core_ibex_reg_pkg.sv | 1 + hw/ip/rv_dm/rtl/rv_dm_reg_pkg.sv | 1 + hw/ip/spi_device/rtl/spi_device_reg_pkg.sv | 1 + hw/ip/spi_host/rtl/spi_host_reg_pkg.sv | 2 ++ hw/ip/usbdev/rtl/usbdev_reg_pkg.sv | 1 + .../ip/flash_ctrl/rtl/autogen/flash_ctrl_reg_pkg.sv | 2 ++ util/reggen/reg_pkg.sv.tpl | 3 +++ 13 files changed, 20 insertions(+) diff --git a/hw/ip/flash_ctrl/rtl/flash_ctrl_reg_pkg.sv b/hw/ip/flash_ctrl/rtl/flash_ctrl_reg_pkg.sv index a019aa31265d3..e479c20c78b31 100644 --- a/hw/ip/flash_ctrl/rtl/flash_ctrl_reg_pkg.sv +++ b/hw/ip/flash_ctrl/rtl/flash_ctrl_reg_pkg.sv @@ -925,8 +925,10 @@ package flash_ctrl_reg_pkg; // Window parameters for core interface parameter logic [CoreAw-1:0] FLASH_CTRL_PROG_FIFO_OFFSET = 9'h 1b0; parameter int unsigned FLASH_CTRL_PROG_FIFO_SIZE = 'h 4; + parameter int unsigned FLASH_CTRL_PROG_FIFO_IDX = 0; parameter logic [CoreAw-1:0] FLASH_CTRL_RD_FIFO_OFFSET = 9'h 1b4; parameter int unsigned FLASH_CTRL_RD_FIFO_SIZE = 'h 4; + parameter int unsigned FLASH_CTRL_RD_FIFO_IDX = 1; // Register index for core interface typedef enum int { diff --git a/hw/ip/hmac/rtl/hmac_reg_pkg.sv b/hw/ip/hmac/rtl/hmac_reg_pkg.sv index 6f6a97de4b297..01c8701bc82a2 100644 --- a/hw/ip/hmac/rtl/hmac_reg_pkg.sv +++ b/hw/ip/hmac/rtl/hmac_reg_pkg.sv @@ -253,6 +253,7 @@ package hmac_reg_pkg; // Window parameters parameter logic [BlockAw-1:0] HMAC_MSG_FIFO_OFFSET = 12'h 800; parameter int unsigned HMAC_MSG_FIFO_SIZE = 'h 800; + parameter int unsigned HMAC_MSG_FIFO_IDX = 0; // Register index typedef enum int { diff --git a/hw/ip/kmac/rtl/kmac_reg_pkg.sv b/hw/ip/kmac/rtl/kmac_reg_pkg.sv index 9b3e3a164b462..2a9849bfacc8a 100644 --- a/hw/ip/kmac/rtl/kmac_reg_pkg.sv +++ b/hw/ip/kmac/rtl/kmac_reg_pkg.sv @@ -377,8 +377,10 @@ package kmac_reg_pkg; // Window parameters parameter logic [BlockAw-1:0] KMAC_STATE_OFFSET = 12'h 400; parameter int unsigned KMAC_STATE_SIZE = 'h 200; + parameter int unsigned KMAC_STATE_IDX = 0; parameter logic [BlockAw-1:0] KMAC_MSG_FIFO_OFFSET = 12'h 800; parameter int unsigned KMAC_MSG_FIFO_SIZE = 'h 800; + parameter int unsigned KMAC_MSG_FIFO_IDX = 1; // Register index typedef enum int { diff --git a/hw/ip/otbn/rtl/otbn_reg_pkg.sv b/hw/ip/otbn/rtl/otbn_reg_pkg.sv index 1c98d7502fd5e..63985ab954197 100644 --- a/hw/ip/otbn/rtl/otbn_reg_pkg.sv +++ b/hw/ip/otbn/rtl/otbn_reg_pkg.sv @@ -307,8 +307,10 @@ package otbn_reg_pkg; // Window parameters parameter logic [BlockAw-1:0] OTBN_IMEM_OFFSET = 16'h 4000; parameter int unsigned OTBN_IMEM_SIZE = 'h 1000; + parameter int unsigned OTBN_IMEM_IDX = 0; parameter logic [BlockAw-1:0] OTBN_DMEM_OFFSET = 16'h 8000; parameter int unsigned OTBN_DMEM_SIZE = 'h c00; + parameter int unsigned OTBN_DMEM_IDX = 1; // Register index typedef enum int { diff --git a/hw/ip/otp_ctrl/rtl/otp_ctrl_reg_pkg.sv b/hw/ip/otp_ctrl/rtl/otp_ctrl_reg_pkg.sv index bc3842c085bc0..cf7c2b06bda3f 100644 --- a/hw/ip/otp_ctrl/rtl/otp_ctrl_reg_pkg.sv +++ b/hw/ip/otp_ctrl/rtl/otp_ctrl_reg_pkg.sv @@ -579,6 +579,7 @@ package otp_ctrl_reg_pkg; // Window parameters for core interface parameter logic [CoreAw-1:0] OTP_CTRL_SW_CFG_WINDOW_OFFSET = 13'h 1000; parameter int unsigned OTP_CTRL_SW_CFG_WINDOW_SIZE = 'h 800; + parameter int unsigned OTP_CTRL_SW_CFG_WINDOW_IDX = 0; // Register index for core interface typedef enum int { diff --git a/hw/ip/rom_ctrl/rtl/rom_ctrl_reg_pkg.sv b/hw/ip/rom_ctrl/rtl/rom_ctrl_reg_pkg.sv index 8f84f98a1af26..aa20b221f3b95 100644 --- a/hw/ip/rom_ctrl/rtl/rom_ctrl_reg_pkg.sv +++ b/hw/ip/rom_ctrl/rtl/rom_ctrl_reg_pkg.sv @@ -136,5 +136,6 @@ package rom_ctrl_reg_pkg; // Window parameters for rom interface parameter logic [RomAw-1:0] ROM_CTRL_ROM_OFFSET = 15'h 0; parameter int unsigned ROM_CTRL_ROM_SIZE = 'h 8000; + parameter int unsigned ROM_CTRL_ROM_IDX = 0; endpackage diff --git a/hw/ip/rv_core_ibex/rtl/rv_core_ibex_reg_pkg.sv b/hw/ip/rv_core_ibex/rtl/rv_core_ibex_reg_pkg.sv index 3f58ae20b2b5c..bab83ba405023 100644 --- a/hw/ip/rv_core_ibex/rtl/rv_core_ibex_reg_pkg.sv +++ b/hw/ip/rv_core_ibex/rtl/rv_core_ibex_reg_pkg.sv @@ -215,6 +215,7 @@ package rv_core_ibex_reg_pkg; // Window parameters for cfg interface parameter logic [CfgAw-1:0] RV_CORE_IBEX_DV_SIM_WINDOW_OFFSET = 8'h 80; parameter int unsigned RV_CORE_IBEX_DV_SIM_WINDOW_SIZE = 'h 20; + parameter int unsigned RV_CORE_IBEX_DV_SIM_WINDOW_IDX = 0; // Register index for cfg interface typedef enum int { diff --git a/hw/ip/rv_dm/rtl/rv_dm_reg_pkg.sv b/hw/ip/rv_dm/rtl/rv_dm_reg_pkg.sv index eb2c96c171e93..d0150bc48073b 100644 --- a/hw/ip/rv_dm/rtl/rv_dm_reg_pkg.sv +++ b/hw/ip/rv_dm/rtl/rv_dm_reg_pkg.sv @@ -400,6 +400,7 @@ package rv_dm_reg_pkg; // Window parameters for mem interface parameter logic [MemAw-1:0] RV_DM_ROM_OFFSET = 12'h 800; parameter int unsigned RV_DM_ROM_SIZE = 'h 800; + parameter int unsigned RV_DM_ROM_IDX = 0; // Register index for mem interface typedef enum int { diff --git a/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv b/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv index 13b1c76c55bfd..d67c3b8b5d489 100644 --- a/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv +++ b/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv @@ -650,6 +650,7 @@ package spi_device_reg_pkg; // Window parameters parameter logic [BlockAw-1:0] SPI_DEVICE_BUFFER_OFFSET = 13'h 1000; parameter int unsigned SPI_DEVICE_BUFFER_SIZE = 'h 1000; + parameter int unsigned SPI_DEVICE_BUFFER_IDX = 0; // Register index typedef enum int { diff --git a/hw/ip/spi_host/rtl/spi_host_reg_pkg.sv b/hw/ip/spi_host/rtl/spi_host_reg_pkg.sv index 587f8ead2a318..69e2aa7b57ebe 100644 --- a/hw/ip/spi_host/rtl/spi_host_reg_pkg.sv +++ b/hw/ip/spi_host/rtl/spi_host_reg_pkg.sv @@ -328,8 +328,10 @@ package spi_host_reg_pkg; // Window parameters parameter logic [BlockAw-1:0] SPI_HOST_RXDATA_OFFSET = 6'h 24; parameter int unsigned SPI_HOST_RXDATA_SIZE = 'h 4; + parameter int unsigned SPI_HOST_RXDATA_IDX = 0; parameter logic [BlockAw-1:0] SPI_HOST_TXDATA_OFFSET = 6'h 28; parameter int unsigned SPI_HOST_TXDATA_SIZE = 'h 4; + parameter int unsigned SPI_HOST_TXDATA_IDX = 1; // Register index typedef enum int { diff --git a/hw/ip/usbdev/rtl/usbdev_reg_pkg.sv b/hw/ip/usbdev/rtl/usbdev_reg_pkg.sv index ebdf4b4f5e134..c5d899fcf921c 100644 --- a/hw/ip/usbdev/rtl/usbdev_reg_pkg.sv +++ b/hw/ip/usbdev/rtl/usbdev_reg_pkg.sv @@ -667,6 +667,7 @@ package usbdev_reg_pkg; // Window parameters parameter logic [BlockAw-1:0] USBDEV_BUFFER_OFFSET = 12'h 800; parameter int unsigned USBDEV_BUFFER_SIZE = 'h 800; + parameter int unsigned USBDEV_BUFFER_IDX = 0; // Register index typedef enum int { diff --git a/hw/top_earlgrey/ip/flash_ctrl/rtl/autogen/flash_ctrl_reg_pkg.sv b/hw/top_earlgrey/ip/flash_ctrl/rtl/autogen/flash_ctrl_reg_pkg.sv index a019aa31265d3..e479c20c78b31 100644 --- a/hw/top_earlgrey/ip/flash_ctrl/rtl/autogen/flash_ctrl_reg_pkg.sv +++ b/hw/top_earlgrey/ip/flash_ctrl/rtl/autogen/flash_ctrl_reg_pkg.sv @@ -925,8 +925,10 @@ package flash_ctrl_reg_pkg; // Window parameters for core interface parameter logic [CoreAw-1:0] FLASH_CTRL_PROG_FIFO_OFFSET = 9'h 1b0; parameter int unsigned FLASH_CTRL_PROG_FIFO_SIZE = 'h 4; + parameter int unsigned FLASH_CTRL_PROG_FIFO_IDX = 0; parameter logic [CoreAw-1:0] FLASH_CTRL_RD_FIFO_OFFSET = 9'h 1b4; parameter int unsigned FLASH_CTRL_RD_FIFO_SIZE = 'h 4; + parameter int unsigned FLASH_CTRL_RD_FIFO_IDX = 1; // Register index for core interface typedef enum int { diff --git a/util/reggen/reg_pkg.sv.tpl b/util/reggen/reg_pkg.sv.tpl index 789471b3dd1c6..e911c2f815601 100644 --- a/util/reggen/reg_pkg.sv.tpl +++ b/util/reggen/reg_pkg.sv.tpl @@ -301,14 +301,17 @@ value = "{}'h {:x}".format(aw, r.offset) offset_type = 'logic [{}-1:0]'.format(aw_name) size_type = 'int unsigned' + idx_type = 'int unsigned' max_type_len = max(len(offset_type), len(size_type)) offset_type += ' ' * (max_type_len - len(offset_type)) size_type += ' ' * (max_type_len - len(size_type)) + idx_type += ' ' * (max_type_len - len(idx_type)) %>\ parameter ${offset_type} ${win_pfx}_OFFSET = ${base_txt_val}; parameter ${size_type} ${win_pfx}_SIZE = ${size_txt_val}; + parameter ${idx_type} ${win_pfx}_IDX = ${i}; % endfor % endif \ From 306834addbeef269de22e1a45f594b2e6febb37b Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Thu, 24 Aug 2023 16:26:58 -0700 Subject: [PATCH 2/7] [prim] Make copies of dual port memory files Signed-off-by: Michael Schaffner --- hw/ip/prim/prim_ram_1r1w_adv.core | 19 + hw/ip/prim/prim_ram_1r1w_async_adv.core | 22 ++ hw/ip/prim/rtl/prim_ram_1r1w_adv.sv | 95 +++++ hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv | 358 ++++++++++++++++++ .../lint/prim_generic_ram_1r1w.vlt | 9 + .../lint/prim_generic_ram_1r1w.waiver | 14 + hw/ip/prim_generic/prim_generic_ram_1r1w.core | 45 +++ .../prim_generic/rtl/prim_generic_ram_1r1w.sv | 107 ++++++ 8 files changed, 669 insertions(+) create mode 100644 hw/ip/prim/prim_ram_1r1w_adv.core create mode 100644 hw/ip/prim/prim_ram_1r1w_async_adv.core create mode 100644 hw/ip/prim/rtl/prim_ram_1r1w_adv.sv create mode 100644 hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv create mode 100644 hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt create mode 100644 hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver create mode 100644 hw/ip/prim_generic/prim_generic_ram_1r1w.core create mode 100644 hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv diff --git a/hw/ip/prim/prim_ram_1r1w_adv.core b/hw/ip/prim/prim_ram_1r1w_adv.core new file mode 100644 index 0000000000000..3ff2dcd8b7163 --- /dev/null +++ b/hw/ip/prim/prim_ram_1r1w_adv.core @@ -0,0 +1,19 @@ +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:prim:ram_2p_adv:0.1" +description: "Dual-port RAM primitive with advanced features" +filesets: + files_rtl: + depend: + - lowrisc:prim:ram_2p_async_adv + files: + - rtl/prim_ram_2p_adv.sv + file_type: systemVerilogSource + +targets: + default: + filesets: + - files_rtl diff --git a/hw/ip/prim/prim_ram_1r1w_async_adv.core b/hw/ip/prim/prim_ram_1r1w_async_adv.core new file mode 100644 index 0000000000000..e5eda9d04ce37 --- /dev/null +++ b/hw/ip/prim/prim_ram_1r1w_async_adv.core @@ -0,0 +1,22 @@ +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:prim:ram_2p_async_adv:0.1" +description: "Asynchronous dual-port RAM primitive with advanced features" +filesets: + files_rtl: + depend: + - lowrisc:prim:assert + - lowrisc:prim:util + - lowrisc:prim:secded + - lowrisc:prim:ram_2p + files: + - rtl/prim_ram_2p_async_adv.sv + file_type: systemVerilogSource + +targets: + default: + filesets: + - files_rtl diff --git a/hw/ip/prim/rtl/prim_ram_1r1w_adv.sv b/hw/ip/prim/rtl/prim_ram_1r1w_adv.sv new file mode 100644 index 0000000000000..866217a64bbc9 --- /dev/null +++ b/hw/ip/prim/rtl/prim_ram_1r1w_adv.sv @@ -0,0 +1,95 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 +// +// Dual-Port SRAM Wrapper +// +// Supported configurations: +// - ECC for 32b and 64b wide memories with no write mask +// (Width == 32 or Width == 64, DataBitsPerMask is ignored). +// - Byte parity if Width is a multiple of 8 bit and write masks have Byte +// granularity (DataBitsPerMask == 8). +// +// Note that the write mask needs to be per Byte if parity is enabled. If ECC is enabled, the write +// mask cannot be used and has to be tied to {Width{1'b1}}. + +`include "prim_assert.sv" + +module prim_ram_2p_adv import prim_ram_2p_pkg::*; #( + parameter int Depth = 512, + parameter int Width = 32, + parameter int DataBitsPerMask = 1, // Number of data bits per bit of write mask + parameter MemInitFile = "", // VMEM file to initialize the memory with + + // Configurations + parameter bit EnableECC = 0, // Enables per-word ECC + parameter bit EnableParity = 0, // Enables per-Byte Parity + parameter bit EnableInputPipeline = 0, // Adds an input register (read latency +1) + parameter bit EnableOutputPipeline = 0, // Adds an output register (read latency +1) + + // This switch allows to switch to standard Hamming ECC instead of the HSIAO ECC. + // It is recommended to leave this parameter at its default setting (HSIAO), + // since this results in a more compact and faster implementation. + parameter bit HammingECC = 0, + + localparam int Aw = prim_util_pkg::vbits(Depth) +) ( + input clk_i, + input rst_ni, + + input a_req_i, + input a_write_i, + input [Aw-1:0] a_addr_i, + input [Width-1:0] a_wdata_i, + input [Width-1:0] a_wmask_i, // cannot be used with ECC, tie to 1 in that case + output logic [Width-1:0] a_rdata_o, + output logic a_rvalid_o, // read response (a_rdata_o) is valid + output logic [1:0] a_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable + + input b_req_i, + input b_write_i, + input [Aw-1:0] b_addr_i, + input [Width-1:0] b_wdata_i, + input [Width-1:0] b_wmask_i, // cannot be used with ECC, tie to 1 in that case + output logic [Width-1:0] b_rdata_o, + output logic b_rvalid_o, // read response (b_rdata_o) is valid + output logic [1:0] b_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable + + input ram_2p_cfg_t cfg_i +); + + prim_ram_2p_async_adv #( + .Depth (Depth), + .Width (Width), + .DataBitsPerMask (DataBitsPerMask), + .MemInitFile (MemInitFile), + .EnableECC (EnableECC), + .EnableParity (EnableParity), + .EnableInputPipeline (EnableInputPipeline), + .EnableOutputPipeline(EnableOutputPipeline), + .HammingECC (HammingECC) + ) i_prim_ram_2p_async_adv ( + .clk_a_i(clk_i), + .rst_a_ni(rst_ni), + .clk_b_i(clk_i), + .rst_b_ni(rst_ni), + .a_req_i, + .a_write_i, + .a_addr_i, + .a_wdata_i, + .a_wmask_i, + .a_rdata_o, + .a_rvalid_o, + .a_rerror_o, + .b_req_i, + .b_write_i, + .b_addr_i, + .b_wdata_i, + .b_wmask_i, + .b_rdata_o, + .b_rvalid_o, + .b_rerror_o, + .cfg_i + ); + +endmodule : prim_ram_2p_adv diff --git a/hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv b/hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv new file mode 100644 index 0000000000000..d3de6f67d930d --- /dev/null +++ b/hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv @@ -0,0 +1,358 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 +// +// Asynchronous Dual-Port SRAM Wrapper +// +// Supported configurations: +// - ECC for 32b and 64b wide memories with no write mask +// (Width == 32 or Width == 64, DataBitsPerMask is ignored). +// - Byte parity if Width is a multiple of 8 bit and write masks have Byte +// granularity (DataBitsPerMask == 8). +// +// Note that the write mask needs to be per Byte if parity is enabled. If ECC is enabled, the write +// mask cannot be used and has to be tied to {Width{1'b1}}. + +`include "prim_assert.sv" + +module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( + parameter int Depth = 512, + parameter int Width = 32, + parameter int DataBitsPerMask = 1, // Number of data bits per bit of write mask + parameter MemInitFile = "", // VMEM file to initialize the memory with + + // Configurations + parameter bit EnableECC = 0, // Enables per-word ECC + parameter bit EnableParity = 0, // Enables per-Byte Parity + parameter bit EnableInputPipeline = 0, // Adds an input register (read latency +1) + parameter bit EnableOutputPipeline = 0, // Adds an output register (read latency +1) + + // This switch allows to switch to standard Hamming ECC instead of the HSIAO ECC. + // It is recommended to leave this parameter at its default setting (HSIAO), + // since this results in a more compact and faster implementation. + parameter bit HammingECC = 0, + + localparam int Aw = prim_util_pkg::vbits(Depth) +) ( + input clk_a_i, + input clk_b_i, + input rst_a_ni, + input rst_b_ni, + + input a_req_i, + input a_write_i, + input [Aw-1:0] a_addr_i, + input [Width-1:0] a_wdata_i, + input [Width-1:0] a_wmask_i, // cannot be used with ECC, tie to 1 in that case + output logic [Width-1:0] a_rdata_o, + output logic a_rvalid_o, // read response (a_rdata_o) is valid + output logic [1:0] a_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable + + input b_req_i, + input b_write_i, + input [Aw-1:0] b_addr_i, + input [Width-1:0] b_wdata_i, + input [Width-1:0] b_wmask_i, // cannot be used with ECC, tie to 1 in that case + output logic [Width-1:0] b_rdata_o, + output logic b_rvalid_o, // read response (b_rdata_o) is valid + output logic [1:0] b_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable + + // config + input ram_2p_cfg_t cfg_i +); + + + `ASSERT_INIT(CannotHaveEccAndParity_A, !(EnableParity && EnableECC)) + + // Calculate ECC width + localparam int ParWidth = (EnableParity) ? Width/8 : + (!EnableECC) ? 0 : + (Width <= 4) ? 4 : + (Width <= 11) ? 5 : + (Width <= 26) ? 6 : + (Width <= 57) ? 7 : + (Width <= 120) ? 8 : 8 ; + localparam int TotalWidth = Width + ParWidth; + + // If byte parity is enabled, the write enable bits are used to write memory colums + // with 8 + 1 = 9 bit width (data plus corresponding parity bit). + // If ECC is enabled, the DataBitsPerMask is ignored. + localparam int LocalDataBitsPerMask = (EnableParity) ? 9 : + (EnableECC) ? TotalWidth : + DataBitsPerMask; + + //////////////////////////// + // RAM Primitive Instance // + //////////////////////////// + + logic a_req_q, a_req_d ; + logic a_write_q, a_write_d ; + logic [Aw-1:0] a_addr_q, a_addr_d ; + logic [TotalWidth-1:0] a_wdata_q, a_wdata_d ; + logic [TotalWidth-1:0] a_wmask_q, a_wmask_d ; + logic a_rvalid_q, a_rvalid_d, a_rvalid_sram_q ; + logic [Width-1:0] a_rdata_q, a_rdata_d ; + logic [TotalWidth-1:0] a_rdata_sram ; + logic [1:0] a_rerror_q, a_rerror_d ; + + logic b_req_q, b_req_d ; + logic b_write_q, b_write_d ; + logic [Aw-1:0] b_addr_q, b_addr_d ; + logic [TotalWidth-1:0] b_wdata_q, b_wdata_d ; + logic [TotalWidth-1:0] b_wmask_q, b_wmask_d ; + logic b_rvalid_q, b_rvalid_d, b_rvalid_sram_q ; + logic [Width-1:0] b_rdata_q, b_rdata_d ; + logic [TotalWidth-1:0] b_rdata_sram ; + logic [1:0] b_rerror_q, b_rerror_d ; + + prim_ram_2p #( + .MemInitFile (MemInitFile), + + .Width (TotalWidth), + .Depth (Depth), + .DataBitsPerMask (LocalDataBitsPerMask) + ) u_mem ( + .clk_a_i (clk_a_i), + .clk_b_i (clk_b_i), + + .a_req_i (a_req_q), + .a_write_i (a_write_q), + .a_addr_i (a_addr_q), + .a_wdata_i (a_wdata_q), + .a_wmask_i (a_wmask_q), + .a_rdata_o (a_rdata_sram), + + .b_req_i (b_req_q), + .b_write_i (b_write_q), + .b_addr_i (b_addr_q), + .b_wdata_i (b_wdata_q), + .b_wmask_i (b_wmask_q), + .b_rdata_o (b_rdata_sram), + + .cfg_i + ); + + always_ff @(posedge clk_a_i or negedge rst_a_ni) begin + if (!rst_a_ni) begin + a_rvalid_sram_q <= 1'b0; + end else begin + a_rvalid_sram_q <= a_req_q & ~a_write_q; + end + end + always_ff @(posedge clk_b_i or negedge rst_b_ni) begin + if (!rst_b_ni) begin + b_rvalid_sram_q <= 1'b0; + end else begin + b_rvalid_sram_q <= b_req_q & ~b_write_q; + end + end + + assign a_req_d = a_req_i; + assign a_write_d = a_write_i; + assign a_addr_d = a_addr_i; + assign a_rvalid_o = a_rvalid_q; + assign a_rdata_o = a_rdata_q; + assign a_rerror_o = a_rerror_q; + + assign b_req_d = b_req_i; + assign b_write_d = b_write_i; + assign b_addr_d = b_addr_i; + assign b_rvalid_o = b_rvalid_q; + assign b_rdata_o = b_rdata_q; + assign b_rerror_o = b_rerror_q; + + ///////////////////////////// + // ECC / Parity Generation // + ///////////////////////////// + + if (EnableParity == 0 && EnableECC) begin : gen_secded + + // check supported widths + `ASSERT_INIT(SecDecWidth_A, Width inside {32}) + + // the wmask is constantly set to 1 in this case + `ASSERT(OnlyWordWritePossibleWithEccPortA_A, a_req_i |-> + a_wmask_i == {Width{1'b1}}, clk_a_i, rst_a_ni) + `ASSERT(OnlyWordWritePossibleWithEccPortB_A, b_req_i |-> + b_wmask_i == {Width{1'b1}}, clk_b_i, rst_b_ni) + + assign a_wmask_d = {TotalWidth{1'b1}}; + assign b_wmask_d = {TotalWidth{1'b1}}; + + if (Width == 32) begin : gen_secded_39_32 + if (HammingECC) begin : gen_hamming + prim_secded_inv_hamming_39_32_enc u_enc_a ( + .data_i(a_wdata_i), + .data_o(a_wdata_d) + ); + prim_secded_inv_hamming_39_32_dec u_dec_a ( + .data_i (a_rdata_sram), + .data_o (a_rdata_d[0+:Width]), + .syndrome_o ( ), + .err_o (a_rerror_d) + ); + prim_secded_inv_hamming_39_32_enc u_enc_b ( + .data_i(b_wdata_i), + .data_o(b_wdata_d) + ); + prim_secded_inv_hamming_39_32_dec u_dec_b ( + .data_i (b_rdata_sram), + .data_o (b_rdata_d[0+:Width]), + .syndrome_o ( ), + .err_o (b_rerror_d) + ); + end else begin : gen_hsiao + prim_secded_inv_39_32_enc u_enc_a ( + .data_i(a_wdata_i), + .data_o(a_wdata_d) + ); + prim_secded_inv_39_32_dec u_dec_a ( + .data_i (a_rdata_sram), + .data_o (a_rdata_d[0+:Width]), + .syndrome_o ( ), + .err_o (a_rerror_d) + ); + prim_secded_inv_39_32_enc u_enc_b ( + .data_i(b_wdata_i), + .data_o(b_wdata_d) + ); + prim_secded_inv_39_32_dec u_dec_b ( + .data_i (b_rdata_sram), + .data_o (b_rdata_d[0+:Width]), + .syndrome_o ( ), + .err_o (b_rerror_d) + ); + end + end + end else if (EnableParity) begin : gen_byte_parity + + `ASSERT_INIT(ParityNeedsByteWriteMask_A, DataBitsPerMask == 8) + `ASSERT_INIT(WidthNeedsToBeByteAligned_A, Width % 8 == 0) + + always_comb begin : p_parity + a_rerror_d = '0; + b_rerror_d = '0; + for (int i = 0; i < Width/8; i ++) begin + // Data mapping. We have to make 8+1 = 9 bit groups + // that have the same write enable such that FPGA tools + // can map this correctly to BRAM resources. + a_wmask_d[i*9 +: 8] = a_wmask_i[i*8 +: 8]; + a_wdata_d[i*9 +: 8] = a_wdata_i[i*8 +: 8]; + a_rdata_d[i*8 +: 8] = a_rdata_sram[i*9 +: 8]; + b_wmask_d[i*9 +: 8] = b_wmask_i[i*8 +: 8]; + b_wdata_d[i*9 +: 8] = b_wdata_i[i*8 +: 8]; + b_rdata_d[i*8 +: 8] = b_rdata_sram[i*9 +: 8]; + + // parity generation (odd parity) + a_wdata_d[i*9 + 8] = ~(^a_wdata_i[i*8 +: 8]); + a_wmask_d[i*9 + 8] = &a_wmask_i[i*8 +: 8]; + b_wdata_d[i*9 + 8] = ~(^b_wdata_i[i*8 +: 8]); + b_wmask_d[i*9 + 8] = &b_wmask_i[i*8 +: 8]; + // parity decoding (errors are always uncorrectable) + a_rerror_d[1] |= ~(^{a_rdata_sram[i*9 +: 8], a_rdata_sram[i*9 + 8]}); + b_rerror_d[1] |= ~(^{b_rdata_sram[i*9 +: 8], b_rdata_sram[i*9 + 8]}); + end + end + end else begin : gen_nosecded_noparity + assign a_wmask_d = a_wmask_i; + assign b_wmask_d = b_wmask_i; + assign a_wdata_d = a_wdata_i; + assign b_wdata_d = b_wdata_i; + assign a_rdata_d = a_rdata_sram[0+:Width]; + assign b_rdata_d = b_rdata_sram[0+:Width]; + assign a_rerror_d = '0; + assign b_rerror_d = '0; + end + + assign a_rvalid_d = a_rvalid_sram_q; + assign b_rvalid_d = b_rvalid_sram_q; + + ///////////////////////////////////// + // Input/Output Pipeline Registers // + ///////////////////////////////////// + + if (EnableInputPipeline) begin : gen_regslice_input + // Put the register slices between ECC encoding to SRAM port + always_ff @(posedge clk_a_i or negedge rst_a_ni) begin + if (!rst_a_ni) begin + a_req_q <= '0; + a_write_q <= '0; + a_addr_q <= '0; + a_wdata_q <= '0; + a_wmask_q <= '0; + end else begin + a_req_q <= a_req_d; + a_write_q <= a_write_d; + a_addr_q <= a_addr_d; + a_wdata_q <= a_wdata_d; + a_wmask_q <= a_wmask_d; + end + end + always_ff @(posedge clk_b_i or negedge rst_b_ni) begin + if (!rst_b_ni) begin + b_req_q <= '0; + b_write_q <= '0; + b_addr_q <= '0; + b_wdata_q <= '0; + b_wmask_q <= '0; + end else begin + b_req_q <= b_req_d; + b_write_q <= b_write_d; + b_addr_q <= b_addr_d; + b_wdata_q <= b_wdata_d; + b_wmask_q <= b_wmask_d; + end + end + end else begin : gen_dirconnect_input + assign a_req_q = a_req_d; + assign a_write_q = a_write_d; + assign a_addr_q = a_addr_d; + assign a_wdata_q = a_wdata_d; + assign a_wmask_q = a_wmask_d; + + assign b_req_q = b_req_d; + assign b_write_q = b_write_d; + assign b_addr_q = b_addr_d; + assign b_wdata_q = b_wdata_d; + assign b_wmask_q = b_wmask_d; + end + + if (EnableOutputPipeline) begin : gen_regslice_output + // Put the register slices between ECC decoding to output + always_ff @(posedge clk_a_i or negedge rst_a_ni) begin + if (!rst_a_ni) begin + a_rvalid_q <= '0; + a_rdata_q <= '0; + a_rerror_q <= '0; + end else begin + a_rvalid_q <= a_rvalid_d; + a_rdata_q <= a_rdata_d; + // tie to zero if the read data is not valid + a_rerror_q <= a_rerror_d & {2{a_rvalid_d}}; + end + end + always_ff @(posedge clk_b_i or negedge rst_b_ni) begin + if (!rst_b_ni) begin + b_rvalid_q <= '0; + b_rdata_q <= '0; + b_rerror_q <= '0; + end else begin + b_rvalid_q <= b_rvalid_d; + b_rdata_q <= b_rdata_d; + // tie to zero if the read data is not valid + b_rerror_q <= b_rerror_d & {2{b_rvalid_d}}; + end + end + end else begin : gen_dirconnect_output + assign a_rvalid_q = a_rvalid_d; + assign a_rdata_q = a_rdata_d; + // tie to zero if the read data is not valid + assign a_rerror_q = a_rerror_d & {2{a_rvalid_d}}; + + assign b_rvalid_q = b_rvalid_d; + assign b_rdata_q = b_rdata_d; + // tie to zero if the read data is not valid + assign b_rerror_q = b_rerror_d & {2{b_rvalid_d}}; + end + +endmodule : prim_ram_2p_async_adv diff --git a/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt new file mode 100644 index 0000000000000..2654984e20a8d --- /dev/null +++ b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt @@ -0,0 +1,9 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 +// + +`verilator_config + +// That is the nature of a dual-port memory: both write ports can access the same storage simultaneously. +lint_off -rule MULTIDRIVEN -file "*/rtl/prim_generic_ram_2p.sv" -match "Signal has multiple driving blocks with different clocking: '*.mem'*" diff --git a/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver new file mode 100644 index 0000000000000..d6f7d5e7114c4 --- /dev/null +++ b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver @@ -0,0 +1,14 @@ +# Copyright lowRISC contributors. +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 +# +# waiver file for prim_generic_ram_[1,2]p + +waive -rules MULTI_PROC_ASSIGN -location {prim_generic_ram_2p.sv} -regexp {Assignment to 'mem' from more than one block} \ + -comment "That is the nature of a dual-port memory: both write ports can access the same storage simultaneously" +waive -rules ALWAYS_SPEC -location {prim_generic_ram_*p.sv} -regexp {Edge triggered block may be more accurately modeled as always_ff} \ + -comment "Vivado requires here an always instead of always_ff" +waive -rules HIER_NET_NOT_READ -regexp {Connected net '(addr|wdata)_i' at prim_generic_ram_1p.sv.* is not read from in module 'prim_generic_ram_1p'} \ + -comment "Ascentlint blackboxes very deep RAMs to speed up runtime. This blackboxing causes above lint errors." +waive -rules IFDEF_CODE -location {prim_generic_ram_2p.sv} -regexp {Assignment to 'unused_cfg' contained within `ifndef} \ + -comment "Declaration of signal and assignment to it are in same `ifndef" diff --git a/hw/ip/prim_generic/prim_generic_ram_1r1w.core b/hw/ip/prim_generic/prim_generic_ram_1r1w.core new file mode 100644 index 0000000000000..065962b5b76bc --- /dev/null +++ b/hw/ip/prim_generic/prim_generic_ram_1r1w.core @@ -0,0 +1,45 @@ +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:prim_generic:ram_2p" +description: "prim" +filesets: + files_rtl: + depend: + - lowrisc:prim:assert + - lowrisc:prim:ram_2p_pkg + - lowrisc:prim:util_memload + files: + - rtl/prim_generic_ram_2p.sv + file_type: systemVerilogSource + + files_verilator_waiver: + depend: + # common waivers + - lowrisc:lint:common + files: + - lint/prim_generic_ram_2p.vlt + file_type: vlt + + files_ascentlint_waiver: + depend: + # common waivers + - lowrisc:lint:common + files: + - lint/prim_generic_ram_2p.waiver + file_type: waiver + + files_veriblelint_waiver: + depend: + # common waivers + - lowrisc:lint:common + +targets: + default: + filesets: + - tool_verilator ? (files_verilator_waiver) + - tool_ascentlint ? (files_ascentlint_waiver) + - tool_veriblelint ? (files_veriblelint_waiver) + - files_rtl diff --git a/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv b/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv new file mode 100644 index 0000000000000..6e1ebdb57ef28 --- /dev/null +++ b/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv @@ -0,0 +1,107 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 +// +// Synchronous dual-port SRAM register model +// This module is for simulation and small size SRAM. +// Implementing ECC should be done inside wrapper not this model. +`include "prim_assert.sv" +module prim_generic_ram_2p import prim_ram_2p_pkg::*; #( + parameter int Width = 32, // bit + parameter int Depth = 128, + parameter int DataBitsPerMask = 1, // Number of data bits per bit of write mask + parameter MemInitFile = "", // VMEM file to initialize the memory with + + localparam int Aw = $clog2(Depth) // derived parameter +) ( + input clk_a_i, + input clk_b_i, + + input a_req_i, + input a_write_i, + input [Aw-1:0] a_addr_i, + input [Width-1:0] a_wdata_i, + input logic [Width-1:0] a_wmask_i, + output logic [Width-1:0] a_rdata_o, + + + input b_req_i, + input b_write_i, + input [Aw-1:0] b_addr_i, + input [Width-1:0] b_wdata_i, + input logic [Width-1:0] b_wmask_i, + output logic [Width-1:0] b_rdata_o, + + input ram_2p_cfg_t cfg_i +); + +// For certain synthesis experiments we compile the design with generic models to get an unmapped +// netlist (GTECH). In these synthesis experiments, we typically black-box the memory models since +// these are going to be simulated using plain RTL models in netlist simulations. This can be done +// by analyzing and elaborating the design, and then removing the memory submodules before writing +// out the verilog netlist. However, memory arrays can take a long time to elaborate, and in case +// of dual port rams they can even trigger elab errors due to multiple processes writing to the +// same memory variable concurrently. To this end, we exclude the entire logic in this module in +// these runs with the following macro. +`ifndef SYNTHESIS_MEMORY_BLACK_BOXING + + logic unused_cfg; + assign unused_cfg = ^cfg_i; + + // Width of internal write mask. Note *_wmask_i input into the module is always assumed + // to be the full bit mask. + localparam int MaskWidth = Width / DataBitsPerMask; + + logic [Width-1:0] mem [Depth]; + logic [MaskWidth-1:0] a_wmask; + logic [MaskWidth-1:0] b_wmask; + + for (genvar k = 0; k < MaskWidth; k++) begin : gen_wmask + assign a_wmask[k] = &a_wmask_i[k*DataBitsPerMask +: DataBitsPerMask]; + assign b_wmask[k] = &b_wmask_i[k*DataBitsPerMask +: DataBitsPerMask]; + + // Ensure that all mask bits within a group have the same value for a write + `ASSERT(MaskCheckPortA_A, a_req_i && a_write_i |-> + a_wmask_i[k*DataBitsPerMask +: DataBitsPerMask] inside {{DataBitsPerMask{1'b1}}, '0}, + clk_a_i, '0) + `ASSERT(MaskCheckPortB_A, b_req_i && b_write_i |-> + b_wmask_i[k*DataBitsPerMask +: DataBitsPerMask] inside {{DataBitsPerMask{1'b1}}, '0}, + clk_b_i, '0) + end + + // Xilinx FPGA specific Dual-port RAM coding style + // using always instead of always_ff to avoid 'ICPD - illegal combination of drivers' error + // thrown due to 'mem' being driven by two always processes below + always @(posedge clk_a_i) begin + if (a_req_i) begin + if (a_write_i) begin + for (int i=0; i < MaskWidth; i = i + 1) begin + if (a_wmask[i]) begin + mem[a_addr_i][i*DataBitsPerMask +: DataBitsPerMask] <= + a_wdata_i[i*DataBitsPerMask +: DataBitsPerMask]; + end + end + end else begin + a_rdata_o <= mem[a_addr_i]; + end + end + end + + always @(posedge clk_b_i) begin + if (b_req_i) begin + if (b_write_i) begin + for (int i=0; i < MaskWidth; i = i + 1) begin + if (b_wmask[i]) begin + mem[b_addr_i][i*DataBitsPerMask +: DataBitsPerMask] <= + b_wdata_i[i*DataBitsPerMask +: DataBitsPerMask]; + end + end + end else begin + b_rdata_o <= mem[b_addr_i]; + end + end + end + + `include "prim_util_memload.svh" +`endif +endmodule From b36ae73becceca76392583e3c0edff1cdde7947c Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Thu, 24 Aug 2023 19:35:19 -0700 Subject: [PATCH 3/7] [prim] Add two-port memory implementation Signed-off-by: Michael Schaffner --- .../lint/prim_generic_ram_1p.waiver | 4 +- .../lint/prim_generic_ram_1r1w.vlt | 3 -- .../lint/prim_generic_ram_1r1w.waiver | 10 ++--- .../lint/prim_generic_ram_2p.waiver | 6 +-- hw/ip/prim_generic/prim_generic_ram_1r1w.core | 8 ++-- .../prim_generic/rtl/prim_generic_ram_1r1w.sv | 45 +++++-------------- 6 files changed, 24 insertions(+), 52 deletions(-) diff --git a/hw/ip/prim_generic/lint/prim_generic_ram_1p.waiver b/hw/ip/prim_generic/lint/prim_generic_ram_1p.waiver index cf4af6c3f5788..3a08d90bd3262 100644 --- a/hw/ip/prim_generic/lint/prim_generic_ram_1p.waiver +++ b/hw/ip/prim_generic/lint/prim_generic_ram_1p.waiver @@ -2,9 +2,9 @@ # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 # -# waiver file for prim_generic_ram_[1,2]p +# waiver file for prim_generic_ram_1p -waive -rules ALWAYS_SPEC -location {prim_generic_ram_*p.sv} -regexp {Edge triggered block may be more accurately modeled as always_ff} \ +waive -rules ALWAYS_SPEC -location {prim_generic_ram_1p.sv} -regexp {Edge triggered block may be more accurately modeled as always_ff} \ -comment "Vivado requires here an always instead of always_ff" waive -rules HIER_NET_NOT_READ -regexp {Connected net '(addr|wdata)_i' at prim_generic_ram_1p.sv.* is not read from in module 'prim_generic_ram_1p'} \ -comment "Ascentlint blackboxes very deep RAMs to speed up runtime. This blackboxing causes above lint errors." diff --git a/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt index 2654984e20a8d..700355ae31b10 100644 --- a/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt +++ b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.vlt @@ -4,6 +4,3 @@ // `verilator_config - -// That is the nature of a dual-port memory: both write ports can access the same storage simultaneously. -lint_off -rule MULTIDRIVEN -file "*/rtl/prim_generic_ram_2p.sv" -match "Signal has multiple driving blocks with different clocking: '*.mem'*" diff --git a/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver index d6f7d5e7114c4..0827cbed13c3b 100644 --- a/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver +++ b/hw/ip/prim_generic/lint/prim_generic_ram_1r1w.waiver @@ -2,13 +2,11 @@ # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 # -# waiver file for prim_generic_ram_[1,2]p +# waiver file for prim_generic_ram_1r1w -waive -rules MULTI_PROC_ASSIGN -location {prim_generic_ram_2p.sv} -regexp {Assignment to 'mem' from more than one block} \ - -comment "That is the nature of a dual-port memory: both write ports can access the same storage simultaneously" -waive -rules ALWAYS_SPEC -location {prim_generic_ram_*p.sv} -regexp {Edge triggered block may be more accurately modeled as always_ff} \ +waive -rules ALWAYS_SPEC -location {prim_generic_ram_1r1w.sv} -regexp {Edge triggered block may be more accurately modeled as always_ff} \ -comment "Vivado requires here an always instead of always_ff" -waive -rules HIER_NET_NOT_READ -regexp {Connected net '(addr|wdata)_i' at prim_generic_ram_1p.sv.* is not read from in module 'prim_generic_ram_1p'} \ +waive -rules HIER_NET_NOT_READ -regexp {Connected net '(addr|wdata)_i' at prim_generic_ram_1r1w.sv.* is not read from in module 'prim_generic_ram_1r1w'} \ -comment "Ascentlint blackboxes very deep RAMs to speed up runtime. This blackboxing causes above lint errors." -waive -rules IFDEF_CODE -location {prim_generic_ram_2p.sv} -regexp {Assignment to 'unused_cfg' contained within `ifndef} \ +waive -rules IFDEF_CODE -location {prim_generic_ram_1r1w.sv} -regexp {Assignment to 'unused_cfg' contained within `ifndef} \ -comment "Declaration of signal and assignment to it are in same `ifndef" diff --git a/hw/ip/prim_generic/lint/prim_generic_ram_2p.waiver b/hw/ip/prim_generic/lint/prim_generic_ram_2p.waiver index d6f7d5e7114c4..645745209b31f 100644 --- a/hw/ip/prim_generic/lint/prim_generic_ram_2p.waiver +++ b/hw/ip/prim_generic/lint/prim_generic_ram_2p.waiver @@ -2,13 +2,13 @@ # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 # -# waiver file for prim_generic_ram_[1,2]p +# waiver file for prim_generic_ram_2p waive -rules MULTI_PROC_ASSIGN -location {prim_generic_ram_2p.sv} -regexp {Assignment to 'mem' from more than one block} \ -comment "That is the nature of a dual-port memory: both write ports can access the same storage simultaneously" -waive -rules ALWAYS_SPEC -location {prim_generic_ram_*p.sv} -regexp {Edge triggered block may be more accurately modeled as always_ff} \ +waive -rules ALWAYS_SPEC -location {prim_generic_ram_2p.sv} -regexp {Edge triggered block may be more accurately modeled as always_ff} \ -comment "Vivado requires here an always instead of always_ff" -waive -rules HIER_NET_NOT_READ -regexp {Connected net '(addr|wdata)_i' at prim_generic_ram_1p.sv.* is not read from in module 'prim_generic_ram_1p'} \ +waive -rules HIER_NET_NOT_READ -regexp {Connected net '(addr|wdata)_i' at prim_generic_ram_2p.sv.* is not read from in module 'prim_generic_ram_2p'} \ -comment "Ascentlint blackboxes very deep RAMs to speed up runtime. This blackboxing causes above lint errors." waive -rules IFDEF_CODE -location {prim_generic_ram_2p.sv} -regexp {Assignment to 'unused_cfg' contained within `ifndef} \ -comment "Declaration of signal and assignment to it are in same `ifndef" diff --git a/hw/ip/prim_generic/prim_generic_ram_1r1w.core b/hw/ip/prim_generic/prim_generic_ram_1r1w.core index 065962b5b76bc..1aded35855ddf 100644 --- a/hw/ip/prim_generic/prim_generic_ram_1r1w.core +++ b/hw/ip/prim_generic/prim_generic_ram_1r1w.core @@ -3,7 +3,7 @@ CAPI=2: # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 -name: "lowrisc:prim_generic:ram_2p" +name: "lowrisc:prim_generic:ram_1r1w" description: "prim" filesets: files_rtl: @@ -12,7 +12,7 @@ filesets: - lowrisc:prim:ram_2p_pkg - lowrisc:prim:util_memload files: - - rtl/prim_generic_ram_2p.sv + - rtl/prim_generic_ram_1r1w.sv file_type: systemVerilogSource files_verilator_waiver: @@ -20,7 +20,7 @@ filesets: # common waivers - lowrisc:lint:common files: - - lint/prim_generic_ram_2p.vlt + - lint/prim_generic_ram_1r1w.vlt file_type: vlt files_ascentlint_waiver: @@ -28,7 +28,7 @@ filesets: # common waivers - lowrisc:lint:common files: - - lint/prim_generic_ram_2p.waiver + - lint/prim_generic_ram_1r1w.waiver file_type: waiver files_veriblelint_waiver: diff --git a/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv b/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv index 6e1ebdb57ef28..554f447e2b610 100644 --- a/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv +++ b/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv @@ -2,11 +2,11 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 // -// Synchronous dual-port SRAM register model +// Synchronous two-port SRAM register model // This module is for simulation and small size SRAM. // Implementing ECC should be done inside wrapper not this model. `include "prim_assert.sv" -module prim_generic_ram_2p import prim_ram_2p_pkg::*; #( +module prim_generic_ram_1r1w import prim_ram_2p_pkg::*; #( parameter int Width = 32, // bit parameter int Depth = 128, parameter int DataBitsPerMask = 1, // Number of data bits per bit of write mask @@ -17,19 +17,15 @@ module prim_generic_ram_2p import prim_ram_2p_pkg::*; #( input clk_a_i, input clk_b_i, + // Port A can only write input a_req_i, - input a_write_i, input [Aw-1:0] a_addr_i, input [Width-1:0] a_wdata_i, input logic [Width-1:0] a_wmask_i, - output logic [Width-1:0] a_rdata_o, - + // Port B can only read input b_req_i, - input b_write_i, input [Aw-1:0] b_addr_i, - input [Width-1:0] b_wdata_i, - input logic [Width-1:0] b_wmask_i, output logic [Width-1:0] b_rdata_o, input ram_2p_cfg_t cfg_i @@ -40,7 +36,7 @@ module prim_generic_ram_2p import prim_ram_2p_pkg::*; #( // these are going to be simulated using plain RTL models in netlist simulations. This can be done // by analyzing and elaborating the design, and then removing the memory submodules before writing // out the verilog netlist. However, memory arrays can take a long time to elaborate, and in case -// of dual port rams they can even trigger elab errors due to multiple processes writing to the +// of two port rams they can even trigger elab errors due to multiple processes writing to the // same memory variable concurrently. To this end, we exclude the entire logic in this module in // these runs with the following macro. `ifndef SYNTHESIS_MEMORY_BLACK_BOXING @@ -54,51 +50,32 @@ module prim_generic_ram_2p import prim_ram_2p_pkg::*; #( logic [Width-1:0] mem [Depth]; logic [MaskWidth-1:0] a_wmask; - logic [MaskWidth-1:0] b_wmask; - for (genvar k = 0; k < MaskWidth; k++) begin : gen_wmask assign a_wmask[k] = &a_wmask_i[k*DataBitsPerMask +: DataBitsPerMask]; - assign b_wmask[k] = &b_wmask_i[k*DataBitsPerMask +: DataBitsPerMask]; // Ensure that all mask bits within a group have the same value for a write `ASSERT(MaskCheckPortA_A, a_req_i && a_write_i |-> a_wmask_i[k*DataBitsPerMask +: DataBitsPerMask] inside {{DataBitsPerMask{1'b1}}, '0}, clk_a_i, '0) - `ASSERT(MaskCheckPortB_A, b_req_i && b_write_i |-> - b_wmask_i[k*DataBitsPerMask +: DataBitsPerMask] inside {{DataBitsPerMask{1'b1}}, '0}, - clk_b_i, '0) end - // Xilinx FPGA specific Dual-port RAM coding style + // Xilinx FPGA specific Two-port RAM coding style // using always instead of always_ff to avoid 'ICPD - illegal combination of drivers' error // thrown due to 'mem' being driven by two always processes below always @(posedge clk_a_i) begin if (a_req_i) begin - if (a_write_i) begin - for (int i=0; i < MaskWidth; i = i + 1) begin - if (a_wmask[i]) begin - mem[a_addr_i][i*DataBitsPerMask +: DataBitsPerMask] <= - a_wdata_i[i*DataBitsPerMask +: DataBitsPerMask]; - end + for (int i=0; i < MaskWidth; i = i + 1) begin + if (a_wmask[i]) begin + mem[a_addr_i][i*DataBitsPerMask +: DataBitsPerMask] <= + a_wdata_i[i*DataBitsPerMask +: DataBitsPerMask]; end - end else begin - a_rdata_o <= mem[a_addr_i]; end end end always @(posedge clk_b_i) begin if (b_req_i) begin - if (b_write_i) begin - for (int i=0; i < MaskWidth; i = i + 1) begin - if (b_wmask[i]) begin - mem[b_addr_i][i*DataBitsPerMask +: DataBitsPerMask] <= - b_wdata_i[i*DataBitsPerMask +: DataBitsPerMask]; - end - end - end else begin - b_rdata_o <= mem[b_addr_i]; - end + b_rdata_o <= mem[b_addr_i]; end end From 7b6163d6ef13243e242e25264a2203d7b1819bef Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Thu, 24 Aug 2023 19:35:37 -0700 Subject: [PATCH 4/7] [prim] Add two-port memory ECC wrappers Signed-off-by: Michael Schaffner --- hw/ip/prim/prim_ram_1r1w_adv.core | 8 +- hw/ip/prim/prim_ram_1r1w_async_adv.core | 8 +- hw/ip/prim/rtl/prim_ram_1r1w_adv.sv | 26 ++---- hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv | 108 ++-------------------- 4 files changed, 22 insertions(+), 128 deletions(-) diff --git a/hw/ip/prim/prim_ram_1r1w_adv.core b/hw/ip/prim/prim_ram_1r1w_adv.core index 3ff2dcd8b7163..218bea9ab66df 100644 --- a/hw/ip/prim/prim_ram_1r1w_adv.core +++ b/hw/ip/prim/prim_ram_1r1w_adv.core @@ -3,14 +3,14 @@ CAPI=2: # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 -name: "lowrisc:prim:ram_2p_adv:0.1" -description: "Dual-port RAM primitive with advanced features" +name: "lowrisc:prim:ram_1r1w_adv:0.1" +description: "Two-port (1 read-only port, 1 write-only port) RAM primitive with advanced features" filesets: files_rtl: depend: - - lowrisc:prim:ram_2p_async_adv + - lowrisc:prim:ram_1r1w_async_adv files: - - rtl/prim_ram_2p_adv.sv + - rtl/prim_ram_1r1w_adv.sv file_type: systemVerilogSource targets: diff --git a/hw/ip/prim/prim_ram_1r1w_async_adv.core b/hw/ip/prim/prim_ram_1r1w_async_adv.core index e5eda9d04ce37..a1caaa65ac943 100644 --- a/hw/ip/prim/prim_ram_1r1w_async_adv.core +++ b/hw/ip/prim/prim_ram_1r1w_async_adv.core @@ -3,17 +3,17 @@ CAPI=2: # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 -name: "lowrisc:prim:ram_2p_async_adv:0.1" -description: "Asynchronous dual-port RAM primitive with advanced features" +name: "lowrisc:prim:ram_1r1w_async_adv:0.1" +description: "Asynchronous two-port (1 read-only port, 1 write-only port) RAM primitive with advanced features" filesets: files_rtl: depend: - lowrisc:prim:assert - lowrisc:prim:util - lowrisc:prim:secded - - lowrisc:prim:ram_2p + - lowrisc:prim:ram_1r1w files: - - rtl/prim_ram_2p_async_adv.sv + - rtl/prim_ram_1r1w_async_adv.sv file_type: systemVerilogSource targets: diff --git a/hw/ip/prim/rtl/prim_ram_1r1w_adv.sv b/hw/ip/prim/rtl/prim_ram_1r1w_adv.sv index 866217a64bbc9..ddcb4f9d87878 100644 --- a/hw/ip/prim/rtl/prim_ram_1r1w_adv.sv +++ b/hw/ip/prim/rtl/prim_ram_1r1w_adv.sv @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 // -// Dual-Port SRAM Wrapper +// Two-Port SRAM Wrapper // // Supported configurations: // - ECC for 32b and 64b wide memories with no write mask @@ -15,7 +15,7 @@ `include "prim_assert.sv" -module prim_ram_2p_adv import prim_ram_2p_pkg::*; #( +module prim_ram_1r1w_adv import prim_ram_2p_pkg::*; #( parameter int Depth = 512, parameter int Width = 32, parameter int DataBitsPerMask = 1, // Number of data bits per bit of write mask @@ -37,20 +37,15 @@ module prim_ram_2p_adv import prim_ram_2p_pkg::*; #( input clk_i, input rst_ni, + // Port A can only write input a_req_i, - input a_write_i, input [Aw-1:0] a_addr_i, input [Width-1:0] a_wdata_i, input [Width-1:0] a_wmask_i, // cannot be used with ECC, tie to 1 in that case - output logic [Width-1:0] a_rdata_o, - output logic a_rvalid_o, // read response (a_rdata_o) is valid - output logic [1:0] a_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable + // Port B can only read input b_req_i, - input b_write_i, input [Aw-1:0] b_addr_i, - input [Width-1:0] b_wdata_i, - input [Width-1:0] b_wmask_i, // cannot be used with ECC, tie to 1 in that case output logic [Width-1:0] b_rdata_o, output logic b_rvalid_o, // read response (b_rdata_o) is valid output logic [1:0] b_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable @@ -58,7 +53,7 @@ module prim_ram_2p_adv import prim_ram_2p_pkg::*; #( input ram_2p_cfg_t cfg_i ); - prim_ram_2p_async_adv #( + prim_ram_1r1w_async_adv #( .Depth (Depth), .Width (Width), .DataBitsPerMask (DataBitsPerMask), @@ -68,28 +63,21 @@ module prim_ram_2p_adv import prim_ram_2p_pkg::*; #( .EnableInputPipeline (EnableInputPipeline), .EnableOutputPipeline(EnableOutputPipeline), .HammingECC (HammingECC) - ) i_prim_ram_2p_async_adv ( + ) i_prim_ram_1r1w_async_adv ( .clk_a_i(clk_i), .rst_a_ni(rst_ni), .clk_b_i(clk_i), .rst_b_ni(rst_ni), .a_req_i, - .a_write_i, .a_addr_i, .a_wdata_i, .a_wmask_i, - .a_rdata_o, - .a_rvalid_o, - .a_rerror_o, .b_req_i, - .b_write_i, .b_addr_i, - .b_wdata_i, - .b_wmask_i, .b_rdata_o, .b_rvalid_o, .b_rerror_o, .cfg_i ); -endmodule : prim_ram_2p_adv +endmodule : prim_ram_1r1w_adv diff --git a/hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv b/hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv index d3de6f67d930d..a6b2bc98585a1 100644 --- a/hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv +++ b/hw/ip/prim/rtl/prim_ram_1r1w_async_adv.sv @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 // -// Asynchronous Dual-Port SRAM Wrapper +// Asynchronous Two-Port SRAM Wrapper // // Supported configurations: // - ECC for 32b and 64b wide memories with no write mask @@ -15,7 +15,7 @@ `include "prim_assert.sv" -module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( +module prim_ram_1r1w_async_adv import prim_ram_2p_pkg::*; #( parameter int Depth = 512, parameter int Width = 32, parameter int DataBitsPerMask = 1, // Number of data bits per bit of write mask @@ -39,20 +39,15 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( input rst_a_ni, input rst_b_ni, + // Port A can only write input a_req_i, - input a_write_i, input [Aw-1:0] a_addr_i, input [Width-1:0] a_wdata_i, input [Width-1:0] a_wmask_i, // cannot be used with ECC, tie to 1 in that case - output logic [Width-1:0] a_rdata_o, - output logic a_rvalid_o, // read response (a_rdata_o) is valid - output logic [1:0] a_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable + // Port B can only read input b_req_i, - input b_write_i, input [Aw-1:0] b_addr_i, - input [Width-1:0] b_wdata_i, - input [Width-1:0] b_wmask_i, // cannot be used with ECC, tie to 1 in that case output logic [Width-1:0] b_rdata_o, output logic b_rvalid_o, // read response (b_rdata_o) is valid output logic [1:0] b_rerror_o, // Bit1: Uncorrectable, Bit0: Correctable @@ -86,26 +81,18 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( //////////////////////////// logic a_req_q, a_req_d ; - logic a_write_q, a_write_d ; logic [Aw-1:0] a_addr_q, a_addr_d ; logic [TotalWidth-1:0] a_wdata_q, a_wdata_d ; logic [TotalWidth-1:0] a_wmask_q, a_wmask_d ; - logic a_rvalid_q, a_rvalid_d, a_rvalid_sram_q ; - logic [Width-1:0] a_rdata_q, a_rdata_d ; - logic [TotalWidth-1:0] a_rdata_sram ; - logic [1:0] a_rerror_q, a_rerror_d ; logic b_req_q, b_req_d ; - logic b_write_q, b_write_d ; logic [Aw-1:0] b_addr_q, b_addr_d ; - logic [TotalWidth-1:0] b_wdata_q, b_wdata_d ; - logic [TotalWidth-1:0] b_wmask_q, b_wmask_d ; logic b_rvalid_q, b_rvalid_d, b_rvalid_sram_q ; logic [Width-1:0] b_rdata_q, b_rdata_d ; logic [TotalWidth-1:0] b_rdata_sram ; logic [1:0] b_rerror_q, b_rerror_d ; - prim_ram_2p #( + prim_ram_1r1w #( .MemInitFile (MemInitFile), .Width (TotalWidth), @@ -116,46 +103,29 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( .clk_b_i (clk_b_i), .a_req_i (a_req_q), - .a_write_i (a_write_q), .a_addr_i (a_addr_q), .a_wdata_i (a_wdata_q), .a_wmask_i (a_wmask_q), - .a_rdata_o (a_rdata_sram), .b_req_i (b_req_q), - .b_write_i (b_write_q), .b_addr_i (b_addr_q), - .b_wdata_i (b_wdata_q), - .b_wmask_i (b_wmask_q), .b_rdata_o (b_rdata_sram), .cfg_i ); - always_ff @(posedge clk_a_i or negedge rst_a_ni) begin - if (!rst_a_ni) begin - a_rvalid_sram_q <= 1'b0; - end else begin - a_rvalid_sram_q <= a_req_q & ~a_write_q; - end - end always_ff @(posedge clk_b_i or negedge rst_b_ni) begin if (!rst_b_ni) begin b_rvalid_sram_q <= 1'b0; end else begin - b_rvalid_sram_q <= b_req_q & ~b_write_q; + b_rvalid_sram_q <= b_req_q; end end assign a_req_d = a_req_i; - assign a_write_d = a_write_i; assign a_addr_d = a_addr_i; - assign a_rvalid_o = a_rvalid_q; - assign a_rdata_o = a_rdata_q; - assign a_rerror_o = a_rerror_q; assign b_req_d = b_req_i; - assign b_write_d = b_write_i; assign b_addr_d = b_addr_i; assign b_rvalid_o = b_rvalid_q; assign b_rdata_o = b_rdata_q; @@ -173,11 +143,8 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( // the wmask is constantly set to 1 in this case `ASSERT(OnlyWordWritePossibleWithEccPortA_A, a_req_i |-> a_wmask_i == {Width{1'b1}}, clk_a_i, rst_a_ni) - `ASSERT(OnlyWordWritePossibleWithEccPortB_A, b_req_i |-> - b_wmask_i == {Width{1'b1}}, clk_b_i, rst_b_ni) assign a_wmask_d = {TotalWidth{1'b1}}; - assign b_wmask_d = {TotalWidth{1'b1}}; if (Width == 32) begin : gen_secded_39_32 if (HammingECC) begin : gen_hamming @@ -185,16 +152,6 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( .data_i(a_wdata_i), .data_o(a_wdata_d) ); - prim_secded_inv_hamming_39_32_dec u_dec_a ( - .data_i (a_rdata_sram), - .data_o (a_rdata_d[0+:Width]), - .syndrome_o ( ), - .err_o (a_rerror_d) - ); - prim_secded_inv_hamming_39_32_enc u_enc_b ( - .data_i(b_wdata_i), - .data_o(b_wdata_d) - ); prim_secded_inv_hamming_39_32_dec u_dec_b ( .data_i (b_rdata_sram), .data_o (b_rdata_d[0+:Width]), @@ -206,16 +163,6 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( .data_i(a_wdata_i), .data_o(a_wdata_d) ); - prim_secded_inv_39_32_dec u_dec_a ( - .data_i (a_rdata_sram), - .data_o (a_rdata_d[0+:Width]), - .syndrome_o ( ), - .err_o (a_rerror_d) - ); - prim_secded_inv_39_32_enc u_enc_b ( - .data_i(b_wdata_i), - .data_o(b_wdata_d) - ); prim_secded_inv_39_32_dec u_dec_b ( .data_i (b_rdata_sram), .data_o (b_rdata_d[0+:Width]), @@ -230,7 +177,6 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( `ASSERT_INIT(WidthNeedsToBeByteAligned_A, Width % 8 == 0) always_comb begin : p_parity - a_rerror_d = '0; b_rerror_d = '0; for (int i = 0; i < Width/8; i ++) begin // Data mapping. We have to make 8+1 = 9 bit groups @@ -238,33 +184,22 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( // can map this correctly to BRAM resources. a_wmask_d[i*9 +: 8] = a_wmask_i[i*8 +: 8]; a_wdata_d[i*9 +: 8] = a_wdata_i[i*8 +: 8]; - a_rdata_d[i*8 +: 8] = a_rdata_sram[i*9 +: 8]; - b_wmask_d[i*9 +: 8] = b_wmask_i[i*8 +: 8]; - b_wdata_d[i*9 +: 8] = b_wdata_i[i*8 +: 8]; b_rdata_d[i*8 +: 8] = b_rdata_sram[i*9 +: 8]; // parity generation (odd parity) a_wdata_d[i*9 + 8] = ~(^a_wdata_i[i*8 +: 8]); a_wmask_d[i*9 + 8] = &a_wmask_i[i*8 +: 8]; - b_wdata_d[i*9 + 8] = ~(^b_wdata_i[i*8 +: 8]); - b_wmask_d[i*9 + 8] = &b_wmask_i[i*8 +: 8]; // parity decoding (errors are always uncorrectable) - a_rerror_d[1] |= ~(^{a_rdata_sram[i*9 +: 8], a_rdata_sram[i*9 + 8]}); b_rerror_d[1] |= ~(^{b_rdata_sram[i*9 +: 8], b_rdata_sram[i*9 + 8]}); end end end else begin : gen_nosecded_noparity assign a_wmask_d = a_wmask_i; - assign b_wmask_d = b_wmask_i; assign a_wdata_d = a_wdata_i; - assign b_wdata_d = b_wdata_i; - assign a_rdata_d = a_rdata_sram[0+:Width]; assign b_rdata_d = b_rdata_sram[0+:Width]; - assign a_rerror_d = '0; assign b_rerror_d = '0; end - assign a_rvalid_d = a_rvalid_sram_q; assign b_rvalid_d = b_rvalid_sram_q; ///////////////////////////////////// @@ -276,13 +211,11 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( always_ff @(posedge clk_a_i or negedge rst_a_ni) begin if (!rst_a_ni) begin a_req_q <= '0; - a_write_q <= '0; a_addr_q <= '0; a_wdata_q <= '0; a_wmask_q <= '0; end else begin a_req_q <= a_req_d; - a_write_q <= a_write_d; a_addr_q <= a_addr_d; a_wdata_q <= a_wdata_d; a_wmask_q <= a_wmask_d; @@ -291,46 +224,24 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( always_ff @(posedge clk_b_i or negedge rst_b_ni) begin if (!rst_b_ni) begin b_req_q <= '0; - b_write_q <= '0; b_addr_q <= '0; - b_wdata_q <= '0; - b_wmask_q <= '0; end else begin b_req_q <= b_req_d; - b_write_q <= b_write_d; b_addr_q <= b_addr_d; - b_wdata_q <= b_wdata_d; - b_wmask_q <= b_wmask_d; end end end else begin : gen_dirconnect_input assign a_req_q = a_req_d; - assign a_write_q = a_write_d; assign a_addr_q = a_addr_d; assign a_wdata_q = a_wdata_d; assign a_wmask_q = a_wmask_d; assign b_req_q = b_req_d; - assign b_write_q = b_write_d; assign b_addr_q = b_addr_d; - assign b_wdata_q = b_wdata_d; - assign b_wmask_q = b_wmask_d; end if (EnableOutputPipeline) begin : gen_regslice_output // Put the register slices between ECC decoding to output - always_ff @(posedge clk_a_i or negedge rst_a_ni) begin - if (!rst_a_ni) begin - a_rvalid_q <= '0; - a_rdata_q <= '0; - a_rerror_q <= '0; - end else begin - a_rvalid_q <= a_rvalid_d; - a_rdata_q <= a_rdata_d; - // tie to zero if the read data is not valid - a_rerror_q <= a_rerror_d & {2{a_rvalid_d}}; - end - end always_ff @(posedge clk_b_i or negedge rst_b_ni) begin if (!rst_b_ni) begin b_rvalid_q <= '0; @@ -344,15 +255,10 @@ module prim_ram_2p_async_adv import prim_ram_2p_pkg::*; #( end end end else begin : gen_dirconnect_output - assign a_rvalid_q = a_rvalid_d; - assign a_rdata_q = a_rdata_d; - // tie to zero if the read data is not valid - assign a_rerror_q = a_rerror_d & {2{a_rvalid_d}}; - assign b_rvalid_q = b_rvalid_d; assign b_rdata_q = b_rdata_d; // tie to zero if the read data is not valid assign b_rerror_q = b_rerror_d & {2{b_rvalid_d}}; end -endmodule : prim_ram_2p_async_adv +endmodule : prim_ram_1r1w_async_adv From d186d1bd95b3499b21750832d5d4f998beae6c27 Mon Sep 17 00:00:00 2001 From: Alexander Williams Date: Fri, 25 Aug 2023 14:54:49 -0700 Subject: [PATCH 5/7] [prim] Fix up 1r1w cores Some core files were missing, and there were also some syntax errors. Signed-off-by: Alexander Williams --- hw/ip/prim/lint/prim_ram_1r1w.waiver | 8 +++ hw/ip/prim/prim_ram_1r1w.core | 50 +++++++++++++++++++ .../prim_generic/rtl/prim_generic_ram_1r1w.sv | 2 +- 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 hw/ip/prim/lint/prim_ram_1r1w.waiver create mode 100644 hw/ip/prim/prim_ram_1r1w.core diff --git a/hw/ip/prim/lint/prim_ram_1r1w.waiver b/hw/ip/prim/lint/prim_ram_1r1w.waiver new file mode 100644 index 0000000000000..b247f1f81d941 --- /dev/null +++ b/hw/ip/prim/lint/prim_ram_1r1w.waiver @@ -0,0 +1,8 @@ +# Copyright lowRISC contributors. +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 +# +# waiver file for prim_ram_1r1w + +waive -rules {STAR_PORT_CONN_USE} -location {prim_ram_1r1w.sv} -regexp {.*wild card port connection encountered on instance.*} \ + -comment "Generated prims may have wildcard connections." diff --git a/hw/ip/prim/prim_ram_1r1w.core b/hw/ip/prim/prim_ram_1r1w.core new file mode 100644 index 0000000000000..3730ff2a7b703 --- /dev/null +++ b/hw/ip/prim/prim_ram_1r1w.core @@ -0,0 +1,50 @@ +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:prim:ram_1r1w" +description: "Random-access memory with 1 read-only port and 1 write-only port" +filesets: + primgen_dep: + depend: + - lowrisc:prim:prim_pkg + - lowrisc:prim:ram_2p_pkg + - lowrisc:prim:primgen + + + files_verilator_waiver: + depend: + # common waivers + - lowrisc:lint:common + files: + file_type: vlt + + files_ascentlint_waiver: + depend: + # common waivers + - lowrisc:lint:common + files: + - lint/prim_ram_1r1w.waiver + file_type: waiver + + files_veriblelint_waiver: + depend: + # common waivers + - lowrisc:lint:common + +generate: + impl: + generator: primgen + parameters: + prim_name: ram_1r1w + +targets: + default: + filesets: + - tool_verilator ? (files_verilator_waiver) + - tool_ascentlint ? (files_ascentlint_waiver) + - tool_veriblelint ? (files_veriblelint_waiver) + - primgen_dep + generate: + - impl diff --git a/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv b/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv index 554f447e2b610..396d506db57c0 100644 --- a/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv +++ b/hw/ip/prim_generic/rtl/prim_generic_ram_1r1w.sv @@ -54,7 +54,7 @@ module prim_generic_ram_1r1w import prim_ram_2p_pkg::*; #( assign a_wmask[k] = &a_wmask_i[k*DataBitsPerMask +: DataBitsPerMask]; // Ensure that all mask bits within a group have the same value for a write - `ASSERT(MaskCheckPortA_A, a_req_i && a_write_i |-> + `ASSERT(MaskCheckPortA_A, a_req_i |-> a_wmask_i[k*DataBitsPerMask +: DataBitsPerMask] inside {{DataBitsPerMask{1'b1}}, '0}, clk_a_i, '0) end From bb22f4cbb136f627414093d2d8d26a4d90775f5c Mon Sep 17 00:00:00 2001 From: Alexander Williams Date: Thu, 4 Jan 2024 09:10:05 -0800 Subject: [PATCH 6/7] [spi_device] Add support for 1r1w RAMs and parity init 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 Co-authored-by: Michael Schaffner --- hw/ip/spi_device/data/spi_device.hjson | 52 +++- hw/ip/spi_device/doc/registers.md | 32 ++- .../env/seq_lib/spi_device_mem_parity_vseq.sv | 23 +- .../env/seq_lib/spi_device_pass_base_vseq.sv | 9 +- .../dv/env/seq_lib/spi_device_ram_cfg_vseq.sv | 17 +- hw/ip/spi_device/dv/env/spi_device_env.core | 1 + hw/ip/spi_device/dv/env/spi_device_env_cfg.sv | 13 +- hw/ip/spi_device/dv/env/spi_device_env_cov.sv | 16 -- hw/ip/spi_device/dv/env/spi_device_env_pkg.sv | 7 +- .../dv/env/spi_device_scoreboard.sv | 8 +- hw/ip/spi_device/dv/tb/tb.sv | 4 +- hw/ip/spi_device/lint/spi_device.waiver | 6 + hw/ip/spi_device/rtl/spi_device.sv | 185 +++++++++----- hw/ip/spi_device/rtl/spi_device_pkg.sv | 26 +- hw/ip/spi_device/rtl/spi_device_reg_pkg.sv | 11 +- hw/ip/spi_device/rtl/spi_device_reg_top.sv | 37 +-- hw/ip/spi_device/rtl/spid_dpram.sv | 236 ++++++++++++++++++ hw/ip/spi_device/spi_device.core | 3 + .../data/autogen/top_earlgrey.gen.hjson | 13 +- hw/top_earlgrey/dv/env/chip_common_pkg.sv | 3 - hw/top_earlgrey/rtl/autogen/top_earlgrey.sv | 4 +- sw/device/lib/dif/dif_spi_device.c | 33 +-- sw/device/lib/dif/dif_spi_device.h | 13 +- sw/device/lib/dif/dif_spi_device_unittest.cc | 25 +- sw/device/lib/testing/spi_device_testutils.c | 12 +- .../silicon_creator/lib/drivers/spi_device.c | 16 +- .../silicon_creator/lib/drivers/spi_device.h | 12 +- .../lib/drivers/spi_device_unittest.cc | 12 +- sw/device/tests/sim_dv/spi_passthrough_test.c | 10 +- 29 files changed, 599 insertions(+), 240 deletions(-) create mode 100644 hw/ip/spi_device/rtl/spid_dpram.sv diff --git a/hw/ip/spi_device/data/spi_device.hjson b/hw/ip/spi_device/data/spi_device.hjson index 24cab7480b6ae..32546d699bfd8 100644 --- a/hw/ip/spi_device/data/spi_device.hjson +++ b/hw/ip/spi_device/data/spi_device.hjson @@ -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" @@ -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 ''' }, }, diff --git a/hw/ip/spi_device/doc/registers.md b/hw/ip/spi_device/doc/registers.md index d4ef0751d6082..280a10fc42cf4 100644 --- a/hw/ip/spi_device/doc/registers.md +++ b/hw/ip/spi_device/doc/registers.md @@ -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 @@ -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. diff --git a/hw/ip/spi_device/dv/env/seq_lib/spi_device_mem_parity_vseq.sv b/hw/ip/spi_device/dv/env/seq_lib/spi_device_mem_parity_vseq.sv index 0c39364023459..36be91baf4c51 100644 --- a/hw/ip/spi_device/dv/env/seq_lib/spi_device_mem_parity_vseq.sv +++ b/hw/ip/spi_device/dv/env/seq_lib/spi_device_mem_parity_vseq.sv @@ -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()), diff --git a/hw/ip/spi_device/dv/env/seq_lib/spi_device_pass_base_vseq.sv b/hw/ip/spi_device/dv/env/seq_lib/spi_device_pass_base_vseq.sv index d1ece254886cb..5677f9e87bb11 100644 --- a/hw/ip/spi_device/dv/env/seq_lib/spi_device_pass_base_vseq.sv +++ b/hw/ip/spi_device/dv/env/seq_lib/spi_device_pass_base_vseq.sv @@ -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 @@ -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 @@ -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 diff --git a/hw/ip/spi_device/dv/env/seq_lib/spi_device_ram_cfg_vseq.sv b/hw/ip/spi_device/dv/env/seq_lib/spi_device_ram_cfg_vseq.sv index afefbb1b06d42..1f3d54943a20b 100644 --- a/hw/ip/spi_device/dv/env/seq_lib/spi_device_ram_cfg_vseq.sv +++ b/hw/ip/spi_device/dv/env/seq_lib/spi_device_ram_cfg_vseq.sv @@ -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 diff --git a/hw/ip/spi_device/dv/env/spi_device_env.core b/hw/ip/spi_device/dv/env/spi_device_env.core index 043439ba896c5..04e0c3aaa3615 100644 --- a/hw/ip/spi_device/dv/env/spi_device_env.core +++ b/hw/ip/spi_device/dv/env/spi_device_env.core @@ -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} diff --git a/hw/ip/spi_device/dv/env/spi_device_env_cfg.sv b/hw/ip/spi_device/dv/env/spi_device_env_cfg.sv index 7f48bc0449dc9..f139bb7e66b47 100644 --- a/hw/ip/spi_device/dv/env/spi_device_env_cfg.sv +++ b/hw/ip/spi_device/dv/env/spi_device_env_cfg.sv @@ -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. @@ -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; diff --git a/hw/ip/spi_device/dv/env/spi_device_env_cov.sv b/hw/ip/spi_device/dv/env/spi_device_env_cov.sv index 6237ec0aed563..3090d412c9643 100644 --- a/hw/ip/spi_device/dv/env/spi_device_env_cov.sv +++ b/hw/ip/spi_device/dv/env/spi_device_env_cov.sv @@ -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 @@ -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(); diff --git a/hw/ip/spi_device/dv/env/spi_device_env_pkg.sv b/hw/ip/spi_device/dv/env/spi_device_env_pkg.sv index 8aa6130168c1e..51ba0342fa873 100644 --- a/hw/ip/spi_device/dv/env/spi_device_env_pkg.sv +++ b/hw/ip/spi_device/dv/env/spi_device_env_pkg.sv @@ -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; diff --git a/hw/ip/spi_device/dv/env/spi_device_scoreboard.sv b/hw/ip/spi_device/dv/env/spi_device_scoreboard.sv index fb658501bc5cb..cc7ba81e1e360 100644 --- a/hw/ip/spi_device/dv/env/spi_device_scoreboard.sv +++ b/hw/ip/spi_device/dv/env/spi_device_scoreboard.sv @@ -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)); @@ -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 diff --git a/hw/ip/spi_device/dv/tb/tb.sv b/hw/ip/spi_device/dv/tb/tb.sv index 232f6d4832ef2..b57e19c657494 100644 --- a/hw/ip/spi_device/dv/tb/tb.sv +++ b/hw/ip/spi_device/dv/tb/tb.sv @@ -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 ), diff --git a/hw/ip/spi_device/lint/spi_device.waiver b/hw/ip/spi_device/lint/spi_device.waiver index 2fd0f236bc03c..f86b89a8e2963 100644 --- a/hw/ip/spi_device/lint/spi_device.waiver +++ b/hw/ip/spi_device/lint/spi_device.waiver @@ -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" @@ -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} \ diff --git a/hw/ip/spi_device/rtl/spi_device.sv b/hw/ip/spi_device/rtl/spi_device.sv index 292ab1b5a2d7a..c3e98068dc2d2 100644 --- a/hw/ip/spi_device/rtl/spi_device.sv +++ b/hw/ip/spi_device/rtl/spi_device.sv @@ -9,8 +9,11 @@ module spi_device import spi_device_reg_pkg::NumAlerts; + import spi_device_reg_pkg::SPI_DEVICE_EGRESS_BUFFER_IDX; + import spi_device_reg_pkg::SPI_DEVICE_INGRESS_BUFFER_IDX; #( - parameter logic [NumAlerts-1:0] AlertAsyncOn = {NumAlerts{1'b1}} + parameter logic [NumAlerts-1:0] AlertAsyncOn = {NumAlerts{1'b1}}, + parameter spi_device_pkg::sram_type_e SramType = spi_device_pkg::DefaultSramType ) ( input clk_i, input rst_ni, @@ -81,8 +84,12 @@ module spi_device spi_device_reg_pkg::spi_device_reg2hw_t reg2hw; spi_device_reg_pkg::spi_device_hw2reg_t hw2reg; - tlul_pkg::tl_h2d_t tl_sram_h2d; - tlul_pkg::tl_d2h_t tl_sram_d2h; + tlul_pkg::tl_h2d_t tl_sram_h2d[2]; + tlul_pkg::tl_d2h_t tl_sram_d2h[2]; + tlul_pkg::tl_h2d_t tl_sram_egress_h2d; + tlul_pkg::tl_d2h_t tl_sram_egress_d2h; + tlul_pkg::tl_h2d_t tl_sram_ingress_h2d; + tlul_pkg::tl_d2h_t tl_sram_ingress_d2h; // Dual-port SRAM Interface: Refer prim_ram_2p_wrapper.sv logic mem_a_req; @@ -118,10 +125,11 @@ module spi_device // Upload related interfaces (SRAM, FIFOs) typedef enum int unsigned { - SysSramFw = 0, - SysSramCmdFifo = 1, - SysSramAddrFifo = 2, - SysSramEnd = 3 + SysSramFwEgress = 0, + SysSramFwIngress = 1, + SysSramCmdFifo = 2, + SysSramAddrFifo = 3, + SysSramEnd = 4 } sys_sram_e; sram_l2m_t sys_sram_l2m [SysSramEnd]; // FW, CMDFIFO, ADDRFIFO @@ -130,7 +138,7 @@ module spi_device // Arbiter among Upload CmdFifo/AddrFifo & FW access logic [SysSramEnd-1:0] sys_sram_req ; logic [SysSramEnd-1:0] sys_sram_gnt ; - logic sys_sram_fw_gnt ; + logic [1:0] sys_sram_fw_gnt ; logic [SramAw-1:0] sys_sram_addr [SysSramEnd]; logic [SysSramEnd-1:0] sys_sram_write ; logic [SramDw-1:0] sys_sram_wdata [SysSramEnd]; @@ -1522,52 +1530,96 @@ module spi_device // Common modules // //////////////////// - logic [SramDw-1:0] sys_sram_l2m_fw_wmask; + logic [SramDw-1:0] sys_sram_l2m_fw_wmask[2]; + assign tl_sram_egress_h2d = tl_sram_h2d[SPI_DEVICE_EGRESS_BUFFER_IDX]; + assign tl_sram_d2h[SPI_DEVICE_EGRESS_BUFFER_IDX] = tl_sram_egress_d2h; + assign tl_sram_ingress_h2d = tl_sram_h2d[SPI_DEVICE_INGRESS_BUFFER_IDX]; + assign tl_sram_d2h[SPI_DEVICE_INGRESS_BUFFER_IDX] = tl_sram_ingress_d2h; tlul_adapter_sram #( .SramAw (SramAw), .SramDw (SramDw), .Outstanding (1), + .ErrOnRead (1), // write-only memory window .ByteAccess (0) - ) u_tlul2sram ( + ) u_tlul2sram_egress ( .clk_i, .rst_ni, - .tl_i (tl_sram_h2d), - .tl_o (tl_sram_d2h), + .tl_i (tl_sram_egress_h2d), + .tl_o (tl_sram_egress_d2h), .en_ifetch_i (prim_mubi_pkg::MuBi4False), - .req_o (sys_sram_l2m[SysSramFw].req), + .req_o (sys_sram_l2m[SysSramFwEgress].req), .req_type_o (), - .gnt_i (sys_sram_fw_gnt), - .we_o (sys_sram_l2m[SysSramFw].we), - .addr_o (sys_sram_l2m[SysSramFw].addr), - .wdata_o (sys_sram_l2m[SysSramFw].wdata), - .wmask_o (sys_sram_l2m_fw_wmask), // Not used + .gnt_i (sys_sram_fw_gnt[SPI_DEVICE_EGRESS_BUFFER_IDX]), + .we_o (sys_sram_l2m[SysSramFwEgress].we), + .addr_o (sys_sram_l2m[SysSramFwEgress].addr), + .wdata_o (sys_sram_l2m[SysSramFwEgress].wdata), + .wmask_o (sys_sram_l2m_fw_wmask[SPI_DEVICE_EGRESS_BUFFER_IDX]), // Not used .intg_error_o(), - .rdata_i (sys_sram_m2l[SysSramFw].rdata), - .rvalid_i (sys_sram_m2l[SysSramFw].rvalid), - .rerror_i (sys_sram_m2l[SysSramFw].rerror) + .rdata_i (sys_sram_m2l[SysSramFwEgress].rdata), + .rvalid_i (sys_sram_m2l[SysSramFwEgress].rvalid), + .rerror_i (sys_sram_m2l[SysSramFwEgress].rerror) ); - assign sys_sram_l2m[SysSramFw].wstrb = sram_mask2strb(sys_sram_l2m_fw_wmask); - for (genvar i = 0 ; i < SysSramEnd ; i++) begin : g_sram_connect - if (i == SysSramFw) begin : g_sram_req_sw + tlul_adapter_sram #( + .SramAw (SramAw), + .SramDw (SramDw), + .Outstanding (1), + .ErrOnWrite (1), // read-only memory window + .ByteAccess (0) + ) u_tlul2sram_ingress ( + .clk_i, + .rst_ni, + + .tl_i (tl_sram_ingress_h2d), + .tl_o (tl_sram_ingress_d2h), + .en_ifetch_i (prim_mubi_pkg::MuBi4False), + .req_o (sys_sram_l2m[SysSramFwIngress].req), + .req_type_o (), + .gnt_i (sys_sram_fw_gnt[SPI_DEVICE_INGRESS_BUFFER_IDX]), + .we_o (sys_sram_l2m[SysSramFwIngress].we), + .addr_o (sys_sram_l2m[SysSramFwIngress].addr), + .wdata_o (sys_sram_l2m[SysSramFwIngress].wdata), + .wmask_o (sys_sram_l2m_fw_wmask[SPI_DEVICE_INGRESS_BUFFER_IDX]), // Not used + .intg_error_o(), + .rdata_i (sys_sram_m2l[SysSramFwIngress].rdata), + .rvalid_i (sys_sram_m2l[SysSramFwIngress].rvalid), + .rerror_i (sys_sram_m2l[SysSramFwIngress].rerror) + ); + assign sys_sram_l2m[SysSramFwEgress].wstrb = + sram_mask2strb(sys_sram_l2m_fw_wmask[SPI_DEVICE_EGRESS_BUFFER_IDX]); + assign sys_sram_l2m[SysSramFwIngress].wstrb = + sram_mask2strb(sys_sram_l2m_fw_wmask[SPI_DEVICE_INGRESS_BUFFER_IDX]); + + logic sys_sram_hw_req; + always_comb begin + sys_sram_hw_req = 1'b0; + for (int unsigned i = 0; i < SysSramEnd; i++) begin + if ((i != SysSramFwEgress) && (i != SysSramFwIngress)) begin + sys_sram_hw_req |= sys_sram_l2m[i].req; + end + end + end + + always_comb begin + for (int unsigned i = 0; i < SysSramEnd; i++) begin + sys_sram_req[i] = sys_sram_l2m[i].req; + end + if (sys_sram_hw_req) begin // Fixed low priority. (Discussed in #10065) // When HW requests the SRAM access, lower the SW requests (and grant) - always_comb begin - sys_sram_req[i] = sys_sram_l2m[i].req; - sys_sram_fw_gnt = sys_sram_gnt[i]; - for (int unsigned j = 0; j < SysSramEnd ; j++) begin - if (i != j && sys_sram_l2m[j].req) begin - sys_sram_req[i] = 1'b 0; - // lower the grant - sys_sram_fw_gnt = 1'b 0; - end - end - end - end else begin : g_sram_req_hw - assign sys_sram_req[i] = sys_sram_l2m[i].req; + sys_sram_req[SysSramFwEgress] = 1'b0; + sys_sram_fw_gnt[SPI_DEVICE_EGRESS_BUFFER_IDX] = 1'b0; + sys_sram_req[SysSramFwIngress] = 1'b0; + sys_sram_fw_gnt[SPI_DEVICE_INGRESS_BUFFER_IDX] = 1'b0; + end else begin + sys_sram_fw_gnt[SPI_DEVICE_EGRESS_BUFFER_IDX] = sys_sram_gnt[SysSramFwEgress]; + sys_sram_fw_gnt[SPI_DEVICE_INGRESS_BUFFER_IDX] = sys_sram_gnt[SysSramFwIngress]; end + end + + for (genvar i = 0 ; i < SysSramEnd ; i++) begin : g_sram_connect assign sys_sram_addr [i] = sys_sram_l2m[i].addr; assign sys_sram_write [i] = sys_sram_l2m[i].we; assign sys_sram_wdata [i] = sys_sram_l2m[i].wdata; @@ -1628,41 +1680,38 @@ module spi_device assign mem_b_m2l.rdata = mem_b_rdata; assign mem_b_m2l.rerror = mem_b_rerror; - prim_ram_2p_async_adv #( - .Depth (SramDepth), - .Width (SramDw), // 32 x 512 --> 2kB - .DataBitsPerMask (8), - + spid_dpram #( + .SramType (SramType), .EnableECC (0), .EnableParity (1), .EnableInputPipeline (0), .EnableOutputPipeline(0) - ) u_memory_2p ( - .clk_a_i (clk_i), - .rst_a_ni (rst_ni), - - .clk_b_i (clk_spi_in_buf), - .rst_b_ni (rst_spi_n), - - .a_req_i (mem_a_req), - .a_write_i (mem_a_write), - .a_addr_i (mem_a_addr), - .a_wdata_i (mem_a_wdata), - .a_wmask_i (mem_a_wmask), - .a_rvalid_o (mem_a_rvalid), - .a_rdata_o (mem_a_rdata), - .a_rerror_o (mem_a_rerror), - - .b_req_i (mem_b_req), - .b_write_i (mem_b_write), - .b_addr_i (mem_b_addr), - .b_wdata_i (mem_b_wdata), - .b_wmask_i (mem_b_wmask), - .b_rvalid_o (mem_b_rvalid), - .b_rdata_o (mem_b_rdata), - .b_rerror_o (mem_b_rerror), - - .cfg_i (ram_cfg_i) + ) u_spid_dpram ( + .clk_sys_i (clk_i), + .rst_sys_ni (rst_ni), + + .clk_spi_i (clk_spi_in_buf), + .rst_spi_ni (rst_spi_n), + + .sys_req_i (mem_a_req), + .sys_write_i (mem_a_write), + .sys_addr_i (mem_a_addr), + .sys_wdata_i (mem_a_wdata), + .sys_wmask_i (mem_a_wmask), + .sys_rvalid_o (mem_a_rvalid), + .sys_rdata_o (mem_a_rdata), + .sys_rerror_o (mem_a_rerror), + + .spi_req_i (mem_b_req), + .spi_write_i (mem_b_write), + .spi_addr_i (mem_b_addr), + .spi_wdata_i (mem_b_wdata), + .spi_wmask_i (mem_b_wmask), + .spi_rvalid_o (mem_b_rvalid), + .spi_rdata_o (mem_b_rdata), + .spi_rerror_o (mem_b_rerror), + + .cfg_i (ram_cfg_i) ); // Register module diff --git a/hw/ip/spi_device/rtl/spi_device_pkg.sv b/hw/ip/spi_device/rtl/spi_device_pkg.sv index 6735de4e4a6f6..13bd08fec403f 100644 --- a/hw/ip/spi_device/rtl/spi_device_pkg.sv +++ b/hw/ip/spi_device/rtl/spi_device_pkg.sv @@ -385,7 +385,13 @@ package spi_device_pkg; CmdResetDevice = 8'h 99 } spi_cmd_e; + typedef enum int unsigned { + SramType2p = 0, // True dual-port SRAM (2 RW ports) + SramType1r1w = 1 // Simple dual-port SRAM (1 RO port + 1 WO port) + } sram_type_e; + // Sram parameters + parameter sram_type_e DefaultSramType = SramType2p; parameter int unsigned SramDw = 32; parameter int unsigned SramStrbW = SramDw/8; parameter int unsigned SramOffsetW = $clog2(SramStrbW); @@ -460,7 +466,17 @@ package spi_device_pkg; endfunction : sram_mask2strb // Calculate each space's base and size - parameter sram_addr_t SramReadBufferIdx = sram_addr_t'(0); + // The egress buffer is located at the base of the SRAM address region, and + // the ingress buffer is higher in the map (controlled by reggen). + import spi_device_reg_pkg::SPI_DEVICE_EGRESS_BUFFER_OFFSET; + import spi_device_reg_pkg::SPI_DEVICE_INGRESS_BUFFER_OFFSET; + parameter int unsigned SramEgressByteOffset = 0; + parameter int unsigned SramIngressByteOffset = + SPI_DEVICE_INGRESS_BUFFER_OFFSET - SPI_DEVICE_EGRESS_BUFFER_OFFSET; + + parameter sram_addr_t SramEgressIdx = + sram_addr_t'(SramEgressByteOffset[$clog2(SramDw / 8) +: SramAw]); + parameter sram_addr_t SramReadBufferIdx = SramEgressIdx; parameter sram_addr_t SramReadBufferSize = sram_addr_t'(SramMsgDepth); parameter sram_addr_t SramMailboxIdx = SramReadBufferIdx + SramReadBufferSize; @@ -469,13 +485,15 @@ package spi_device_pkg; parameter sram_addr_t SramSfdpIdx = SramMailboxIdx + SramMailboxSize; parameter sram_addr_t SramSfdpSize = sram_addr_t'(SramSfdpDepth); - parameter sram_addr_t SramPayloadIdx = SramSfdpIdx + SramSfdpSize; + parameter sram_addr_t SramIngressIdx = + sram_addr_t'(SramIngressByteOffset[$clog2(SramDw / 8) +: SramAw]); + parameter sram_addr_t SramPayloadIdx = SramIngressIdx; parameter sram_addr_t SramPayloadSize = sram_addr_t'(SramPayloadDepth); - parameter sram_addr_t SramCmdFifoIdx = SramPayloadIdx + SramPayloadSize ; + parameter sram_addr_t SramCmdFifoIdx = SramPayloadIdx + SramPayloadSize; parameter sram_addr_t SramCmdFifoSize = sram_addr_t'(SramCmdFifoDepth); - parameter sram_addr_t SramAddrFifoIdx = SramCmdFifoIdx + SramCmdFifoSize ; + parameter sram_addr_t SramAddrFifoIdx = SramCmdFifoIdx + SramCmdFifoSize; parameter sram_addr_t SramAddrFifoSize = sram_addr_t'(SramAddrFifoDepth); // Max BitCount in a transaction diff --git a/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv b/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv index d67c3b8b5d489..d947eab8c52e8 100644 --- a/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv +++ b/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv @@ -8,6 +8,8 @@ package spi_device_reg_pkg; // Param list parameter int unsigned SramDepth = 1024; + parameter int unsigned SramEgressDepth = 832; + parameter int unsigned SramIngressDepth = 96; parameter int unsigned NumCmdInfo = 24; parameter int unsigned NumLocality = 5; parameter int unsigned TpmWrFifoPtrW = 7; @@ -648,9 +650,12 @@ package spi_device_reg_pkg; parameter logic [7:0] SPI_DEVICE_TPM_WRITE_FIFO_RESVAL = 8'h 0; // Window parameters - parameter logic [BlockAw-1:0] SPI_DEVICE_BUFFER_OFFSET = 13'h 1000; - parameter int unsigned SPI_DEVICE_BUFFER_SIZE = 'h 1000; - parameter int unsigned SPI_DEVICE_BUFFER_IDX = 0; + parameter logic [BlockAw-1:0] SPI_DEVICE_EGRESS_BUFFER_OFFSET = 13'h 1000; + parameter int unsigned SPI_DEVICE_EGRESS_BUFFER_SIZE = 'h d00; + parameter int unsigned SPI_DEVICE_EGRESS_BUFFER_IDX = 0; + parameter logic [BlockAw-1:0] SPI_DEVICE_INGRESS_BUFFER_OFFSET = 13'h 1e00; + parameter int unsigned SPI_DEVICE_INGRESS_BUFFER_SIZE = 'h 180; + parameter int unsigned SPI_DEVICE_INGRESS_BUFFER_IDX = 1; // Register index typedef enum int { diff --git a/hw/ip/spi_device/rtl/spi_device_reg_top.sv b/hw/ip/spi_device/rtl/spi_device_reg_top.sv index 5592f67efd60f..88e5d74ffe3d4 100644 --- a/hw/ip/spi_device/rtl/spi_device_reg_top.sv +++ b/hw/ip/spi_device/rtl/spi_device_reg_top.sv @@ -13,8 +13,8 @@ module spi_device_reg_top ( output tlul_pkg::tl_d2h_t tl_o, // Output port for window - output tlul_pkg::tl_h2d_t tl_win_o, - input tlul_pkg::tl_d2h_t tl_win_i, + output tlul_pkg::tl_h2d_t tl_win_o [2], + input tlul_pkg::tl_d2h_t tl_win_i [2], // To HW output spi_device_reg_pkg::spi_device_reg2hw_t reg2hw, // Write @@ -91,29 +91,31 @@ module spi_device_reg_top ( .tl_o(tl_o) ); - tlul_pkg::tl_h2d_t tl_socket_h2d [2]; - tlul_pkg::tl_d2h_t tl_socket_d2h [2]; + tlul_pkg::tl_h2d_t tl_socket_h2d [3]; + tlul_pkg::tl_d2h_t tl_socket_d2h [3]; - logic [0:0] reg_steer; + logic [1:0] reg_steer; // socket_1n connection - assign tl_reg_h2d = tl_socket_h2d[1]; - assign tl_socket_d2h[1] = tl_reg_d2h; + assign tl_reg_h2d = tl_socket_h2d[2]; + assign tl_socket_d2h[2] = tl_reg_d2h; - assign tl_win_o = tl_socket_h2d[0]; - assign tl_socket_d2h[0] = tl_win_i; + assign tl_win_o[0] = tl_socket_h2d[0]; + assign tl_socket_d2h[0] = tl_win_i[0]; + assign tl_win_o[1] = tl_socket_h2d[1]; + assign tl_socket_d2h[1] = tl_win_i[1]; // Create Socket_1n tlul_socket_1n #( - .N (2), + .N (3), .HReqPass (1'b1), .HRspPass (1'b1), - .DReqPass ({2{1'b1}}), - .DRspPass ({2{1'b1}}), + .DReqPass ({3{1'b1}}), + .DRspPass ({3{1'b1}}), .HReqDepth (4'h0), .HRspDepth (4'h0), - .DReqDepth ({2{4'h0}}), - .DRspDepth ({2{4'h0}}), + .DReqDepth ({3{4'h0}}), + .DRspDepth ({3{4'h0}}), .ExplicitErrs (1'b0) ) u_socket ( .clk_i (clk_i), @@ -128,13 +130,14 @@ module spi_device_reg_top ( // Create steering logic always_comb begin reg_steer = - tl_i.a_address[AW-1:0] inside {[4096:8191]} ? 1'd0 : + tl_i.a_address[AW-1:0] inside {[4096:7423]} ? 2'd0 : + tl_i.a_address[AW-1:0] inside {[7680:8063]} ? 2'd1 : // Default set to register - 1'd1; + 2'd2; // Override this in case of an integrity error if (intg_err) begin - reg_steer = 1'd1; + reg_steer = 2'd2; end end diff --git a/hw/ip/spi_device/rtl/spid_dpram.sv b/hw/ip/spi_device/rtl/spid_dpram.sv new file mode 100644 index 0000000000000..0f9a27072ffbc --- /dev/null +++ b/hw/ip/spi_device/rtl/spid_dpram.sv @@ -0,0 +1,236 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 +// +// SPI Device dual-port RAM emulation +// +// This module implements the SPI device "dual-port" RAM's interface using 1r1w +// memories. The memory selection for requests depends on the incoming address. +// The sys port read data always completes, so bad addresses don't prevent +// transaction completion. However, the sys port writes do check addresses. +// +// SPI ports don't check addresses, as the behavior is fixed in hardware. + +module spid_dpram + import prim_ram_2p_pkg::*; + import spi_device_pkg::*; +#( + parameter sram_type_e SramType = DefaultSramType, + parameter bit EnableECC = 0, + parameter bit EnableParity = 0, + parameter bit EnableInputPipeline = 0, + parameter bit EnableOutputPipeline = 0 // Adds an output register (read latency +1) +) ( + input clk_sys_i, + input rst_sys_ni, + + input clk_spi_i, + input rst_spi_ni, + + input sys_req_i, + input sys_write_i, + input [SramAw-1:0] sys_addr_i, + input [SramDw-1:0] sys_wdata_i, + input [SramDw-1:0] sys_wmask_i, + output logic sys_rvalid_o, + output logic [SramDw-1:0] sys_rdata_o, + output logic [1:0] sys_rerror_o, + + input spi_req_i, + input spi_write_i, + input [SramAw-1:0] spi_addr_i, + input [SramDw-1:0] spi_wdata_i, + input [SramDw-1:0] spi_wmask_i, + output logic spi_rvalid_o, + output logic [SramDw-1:0] spi_rdata_o, + output logic [1:0] spi_rerror_o, + + input ram_2p_cfg_t cfg_i + ); + + // SYS Wr, SPI Rd is for eFlash, Mailbox, and SFDP + localparam sram_addr_t Sys2SpiOffset = SramEgressIdx; + localparam int unsigned Sys2SpiMinDepth = SramMsgDepth + SramMailboxDepth + SramSfdpDepth; + localparam int unsigned Sys2SpiAw = $clog2(Sys2SpiMinDepth); + localparam int unsigned Sys2SpiDepth = 1 << Sys2SpiAw; + localparam sram_addr_t Sys2SpiEnd = SramReadBufferIdx + sram_addr_t'(Sys2SpiMinDepth); + + // SYS writes only the Sys2Spi memory. + // Only allow software to write to the write-only locations. Filter by + // address. + logic sys2spi_wr_req; + sram_addr_t sys2spi_wr_addr; + assign sys2spi_wr_req = (sys_addr_i < Sys2SpiEnd) & sys_req_i & sys_write_i; + assign sys2spi_wr_addr = sys_addr_i - Sys2SpiOffset; + + // SPI reads from only the Sys2Spi memory. + logic sys2spi_rd_req; + sram_addr_t sys2spi_rd_addr; + assign sys2spi_rd_req = spi_req_i & !spi_write_i; + assign sys2spi_rd_addr = spi_addr_i - Sys2SpiOffset; + + // SPI Wr, SYS Rd is for payload upload + localparam sram_addr_t Spi2SysOffset = SramIngressIdx; + localparam int unsigned Spi2SysMinDepth = SramPayloadDepth + SramCmdFifoDepth + SramAddrFifoDepth; + localparam int unsigned Spi2SysAw = $clog2(Spi2SysMinDepth); + localparam int unsigned Spi2SysDepth = 1 << Spi2SysAw; + + // SPI writes to only the Spi2Sys memory. + logic spi2sys_wr_req; + sram_addr_t spi2sys_wr_addr; + assign spi2sys_wr_req = spi_req_i & spi_write_i; + assign spi2sys_wr_addr = spi_addr_i - Spi2SysOffset; + + // SYS reads only read from the Spi2Sys memory. + // Allow all reads to complete, so the bus always gets a response, even if + // software chooses to read from write-only addresses. + logic spi2sys_rd_req; + sram_addr_t spi2sys_rd_addr; + assign spi2sys_rd_req = sys_req_i & !sys_write_i; + assign spi2sys_rd_addr = sys_addr_i - Spi2SysOffset; + + // 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 words, 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. + logic [SramDw-1:0] spi_wdata, spi_wmask; + logic [Spi2SysDepth-1:0] initialized_words_d, initialized_words_q; + always_comb begin initialized_words_d = initialized_words_q; + // By default, we just loop through the data and wmask. + spi_wdata = spi_wdata_i; + spi_wmask = spi_wmask_i; + // If the word has not been initialized yet we modify the data and wmask to initialize all bits. + if (spi2sys_wr_req && !initialized_words_q[Spi2SysAw'(spi2sys_wr_addr)]) begin + // Mask data at this point already and set all masked bits to 0. + spi_wdata = spi_wdata_i & spi_wmask_i; + spi_wmask = {SramDw{1'b1}}; + // Mark this word as initialized + initialized_words_d[Spi2SysAw'(spi2sys_wr_addr)] = 1'b1; + end + end + + always_ff @(posedge clk_spi_i or negedge rst_spi_ni) begin : p_spi_regs + if (!rst_spi_ni) begin + initialized_words_q <= '0; + end else begin + initialized_words_q <= initialized_words_d; + end + end + + if (SramType == SramType2p) begin : gen_ram2p + prim_ram_2p_async_adv #( + .Depth (SramDepth), + .Width (SramDw), // 32 x 512 --> 2kB + .DataBitsPerMask (8), + + .EnableECC (0), + .EnableParity (1), + .EnableInputPipeline (0), + .EnableOutputPipeline(0) + ) u_memory_2p ( + .clk_a_i (clk_sys_i), + .rst_a_ni (rst_sys_ni), + + .clk_b_i (clk_spi_i), + .rst_b_ni (rst_spi_ni), + + .a_req_i (sys_req_i), + .a_write_i (sys_write_i), + .a_addr_i (sys_addr_i), + .a_wdata_i (sys_wdata_i), + .a_wmask_i (sys_wmask_i), + .a_rvalid_o (sys_rvalid_o), + .a_rdata_o (sys_rdata_o), + .a_rerror_o (sys_rerror_o), + + .b_req_i (spi_req_i), + .b_write_i (spi_write_i), + .b_addr_i (spi_addr_i), + // Use modified wdata and wmask + .b_wdata_i (spi_wdata), + .b_wmask_i (spi_wmask), + .b_rvalid_o (spi_rvalid_o), + .b_rdata_o (spi_rdata_o), + .b_rerror_o (spi_rerror_o), + + .cfg_i + ); + + logic sys2spi_unused; + assign sys2spi_unused = ^{ + sys2spi_wr_req, + sys2spi_wr_addr, + sys2spi_rd_req, + sys2spi_rd_addr + }; + + logic spi2sys_unused; + assign spi2sys_unused = ^{ + spi2sys_wr_req, + spi2sys_wr_addr, + spi2sys_rd_req, + spi2sys_rd_addr + }; + end else if (SramType == SramType1r1w) begin : gen_ram1r1w + prim_ram_1r1w_async_adv #( + .Depth (Sys2SpiDepth), + .Width (SramDw), + .DataBitsPerMask (8), + + .EnableECC (EnableECC), + .EnableParity (EnableParity), + .EnableInputPipeline (EnableInputPipeline), + .EnableOutputPipeline (EnableOutputPipeline) + ) u_sys2spi_mem ( + .clk_a_i (clk_sys_i), + .clk_b_i (clk_spi_i), + .rst_a_ni (rst_sys_ni), + .rst_b_ni (rst_spi_ni), + .a_req_i (sys2spi_wr_req), + .a_addr_i (Sys2SpiAw'(sys2spi_wr_addr)), + .a_wdata_i (sys_wdata_i), + .a_wmask_i (sys_wmask_i), + + .b_req_i (sys2spi_rd_req), + .b_addr_i (Sys2SpiAw'(sys2spi_rd_addr)), + .b_rdata_o (spi_rdata_o), + .b_rvalid_o (spi_rvalid_o), + .b_rerror_o (spi_rerror_o), + + .cfg_i + ); + + prim_ram_1r1w_async_adv #( + .Depth (Spi2SysDepth), + .Width (SramDw), + .DataBitsPerMask (8), + + .EnableECC (EnableECC), + .EnableParity (EnableParity), + .EnableInputPipeline (EnableInputPipeline), + .EnableOutputPipeline (EnableOutputPipeline) + ) u_spi2sys_mem ( + .clk_a_i (clk_spi_i), + .clk_b_i (clk_sys_i), + .rst_a_ni (rst_spi_ni), + .rst_b_ni (rst_sys_ni), + .a_req_i (spi2sys_wr_req), + .a_addr_i (Spi2SysAw'(spi2sys_wr_addr)), + // Use modified wdata and mask. + .a_wdata_i (spi_wdata), + .a_wmask_i (spi_wmask), + + .b_req_i (spi2sys_rd_req), + .b_addr_i (Spi2SysAw'(spi2sys_rd_addr)), + .b_rdata_o (sys_rdata_o), + .b_rvalid_o (sys_rvalid_o), + .b_rerror_o (sys_rerror_o), + + .cfg_i + ); + end + +endmodule // spid_dpram diff --git a/hw/ip/spi_device/spi_device.core b/hw/ip/spi_device/spi_device.core index e3cb7261f0c6c..b1e2581a2d4bb 100644 --- a/hw/ip/spi_device/spi_device.core +++ b/hw/ip/spi_device/spi_device.core @@ -15,6 +15,8 @@ filesets: - lowrisc:prim:clock_inv - lowrisc:prim:mubi - lowrisc:prim:ram_2p_adv + - lowrisc:prim:ram_2p_pkg + - lowrisc:prim:ram_1r1w_async_adv - lowrisc:prim:edge_detector - lowrisc:prim:rst_sync - lowrisc:ip:lc_ctrl_pkg @@ -23,6 +25,7 @@ filesets: files: - rtl/spi_device_reg_top.sv - rtl/spi_cmdparse.sv + - rtl/spid_dpram.sv - rtl/spid_readsram.sv - rtl/spid_readbuffer.sv - rtl/spi_readcmd.sv diff --git a/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson b/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson index ee6a2ada30cca..e2b73cdbfa70a 100644 --- a/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson +++ b/hw/top_earlgrey/data/autogen/top_earlgrey.gen.hjson @@ -796,7 +796,18 @@ "0" ] param_decl: {} - param_list: [] + memory: {} + param_list: + [ + { + name: SramType + desc: Sram Entries. Word size is 32bit width. + type: spi_device_pkg::sram_type_e + default: spi_device_pkg::DefaultSramType + expose: "true" + name_top: SpiDeviceSramType + } + ] inter_signal_list: [ { diff --git a/hw/top_earlgrey/dv/env/chip_common_pkg.sv b/hw/top_earlgrey/dv/env/chip_common_pkg.sv index 14aff84804b24..37d379585378d 100644 --- a/hw/top_earlgrey/dv/env/chip_common_pkg.sv +++ b/hw/top_earlgrey/dv/env/chip_common_pkg.sv @@ -15,9 +15,6 @@ package chip_common_pkg; parameter dv_utils_pkg::uint NUM_PWM_CHANNELS = pwm_reg_pkg::NOutputs; parameter dv_utils_pkg::uint NUM_PATTGEN_CH = pattgen_agent_pkg::NUM_PATTGEN_CHANNELS; - // Buffer is half of SPI_DEVICE Dual Port SRAM - parameter dv_utils_pkg::uint SPI_FRAME_BYTE_SIZE = spi_device_reg_pkg::SPI_DEVICE_BUFFER_SIZE/2; - // SW constants - use unmapped address space with at least 32 bytes. parameter bit [top_pkg::TL_AW-1:0] SW_DV_START_ADDR = tl_main_pkg::ADDR_SPACE_RV_CORE_IBEX__CFG + rv_core_ibex_reg_pkg::RV_CORE_IBEX_DV_SIM_WINDOW_OFFSET; diff --git a/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv b/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv index 82247aba69167..3a3c049a1d263 100644 --- a/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv +++ b/hw/top_earlgrey/rtl/autogen/top_earlgrey.sv @@ -21,6 +21,7 @@ module top_earlgrey #( // parameters for gpio parameter bit GpioGpioAsyncOn = 1, // parameters for spi_device + parameter spi_device_pkg::sram_type_e SpiDeviceSramType = spi_device_pkg::DefaultSramType, // parameters for i2c0 // parameters for i2c1 // parameters for i2c2 @@ -1164,7 +1165,8 @@ module top_earlgrey #( .rst_ni (rstmgr_aon_resets.rst_lc_io_div4_n[rstmgr_pkg::Domain0Sel]) ); spi_device #( - .AlertAsyncOn(alert_handler_reg_pkg::AsyncOn[5:5]) + .AlertAsyncOn(alert_handler_reg_pkg::AsyncOn[5:5]), + .SramType(SpiDeviceSramType) ) u_spi_device ( // Input diff --git a/sw/device/lib/dif/dif_spi_device.c b/sw/device/lib/dif/dif_spi_device.c index 3136349ce8315..5e1b7c134b401 100644 --- a/sw/device/lib/dif/dif_spi_device.c +++ b/sw/device/lib/dif/dif_spi_device.c @@ -12,8 +12,6 @@ #define DIF_SPI_DEVICE_TPM_FIFO_DEPTH 16 -const uint16_t kDifSpiDeviceBufferLen = SPI_DEVICE_BUFFER_SIZE_BYTES; - enum { kDifSpiDeviceFlashStatusWelBit = 1 }; enum { kDifSpiDeviceEFlashLen = 2048 }; enum { kDifSpiDeviceMailboxLen = 1024 }; @@ -24,7 +22,7 @@ enum { kDifSpiDeviceEFlashOffset = 0, kDifSpiDeviceMailboxOffset = 2048, kDifSpiDeviceSfdpOffset = 3072, - kDifSpiDevicePayloadOffset = 3328, + kDifSpiDevicePayloadOffset = 0, }; /** @@ -653,36 +651,29 @@ static dif_result_t dif_spi_device_get_flash_buffer_info( info->buffer_len = kDifSpiDeviceSfdpLen; info->buffer_offset = kDifSpiDeviceSfdpOffset; break; - case kDifSpiDeviceFlashBufferTypePayload: - info->buffer_len = kDifSpiDevicePayloadLen; - info->buffer_offset = kDifSpiDevicePayloadOffset; - break; default: return kDifBadArg; } return kDifOk; } -dif_result_t dif_spi_device_read_flash_buffer( - dif_spi_device_handle_t *spi, - dif_spi_device_flash_buffer_type_t buffer_type, uint32_t offset, - size_t length, uint8_t *buf) { +dif_result_t dif_spi_device_read_flash_payload_buffer( + dif_spi_device_handle_t *spi, uint32_t offset, size_t length, + uint8_t *buf) { if (spi == NULL || buf == NULL) { return kDifBadArg; } - dif_spi_device_flash_buffer_info_t info; - dif_result_t status = - dif_spi_device_get_flash_buffer_info(buffer_type, &info); - if (status != kDifOk) { - return status; - } + const dif_spi_device_flash_buffer_info_t info = { + .buffer_len = kDifSpiDevicePayloadLen, + .buffer_offset = kDifSpiDevicePayloadOffset, + }; if (offset >= (info.buffer_offset + (ptrdiff_t)info.buffer_len) || length > (info.buffer_offset + (ptrdiff_t)info.buffer_len - (ptrdiff_t)offset)) { return kDifBadArg; } - ptrdiff_t offset_from_base = - SPI_DEVICE_BUFFER_REG_OFFSET + info.buffer_offset + (ptrdiff_t)offset; + ptrdiff_t offset_from_base = SPI_DEVICE_INGRESS_BUFFER_REG_OFFSET + + info.buffer_offset + (ptrdiff_t)offset; mmio_region_memcpy_from_mmio32(spi->dev.base_addr, (uint32_t)offset_from_base, buf, length); return kDifOk; @@ -706,8 +697,8 @@ dif_result_t dif_spi_device_write_flash_buffer( (ptrdiff_t)offset)) { return kDifBadArg; } - ptrdiff_t offset_from_base = - SPI_DEVICE_BUFFER_REG_OFFSET + info.buffer_offset + (ptrdiff_t)offset; + ptrdiff_t offset_from_base = SPI_DEVICE_EGRESS_BUFFER_REG_OFFSET + + info.buffer_offset + (ptrdiff_t)offset; mmio_region_memcpy_to_mmio32(spi->dev.base_addr, (uint32_t)offset_from_base, buf, length); return kDifOk; diff --git a/sw/device/lib/dif/dif_spi_device.h b/sw/device/lib/dif/dif_spi_device.h index e3bb41c6b721a..6e82ce4d221e7 100644 --- a/sw/device/lib/dif/dif_spi_device.h +++ b/sw/device/lib/dif/dif_spi_device.h @@ -392,8 +392,6 @@ typedef enum dif_spi_device_flash_buffer_type { kDifSpiDeviceFlashBufferTypeMailbox, /** SFDP region */ kDifSpiDeviceFlashBufferTypeSfdp, - /** Payload for uploaded commands */ - kDifSpiDeviceFlashBufferTypePayload, /** Count of buffer types */ kDifSpiDeviceFlashBufferTypes, } dif_spi_device_flash_buffer_type_t; @@ -589,22 +587,19 @@ dif_result_t dif_spi_device_pop_flash_address_fifo(dif_spi_device_handle_t *spi, uint32_t *address); /** - * Read data from one of the memories associated with flash / passthrough modes. + * Read data from the payload buffer associated with flash / passthrough modes. * * @param spi A handle to a spi device. - * @param buffer_type An identifier for which memory space to read from. * @param offset The starting offset for read data in the memory. * @param length The length, in bytes, of the data to be copied. * @param[out] buf A pointer to the location where the data should be stored. * @return `kDifBadArg` is any pointers are NULL or the `buffer_type` does not * exist. `kDifOutOfRange` if the requested `offset` and `length` go beyond the - * indicated `buffer_type` region. `kDifOk` otherwise. + * payload buffer region. `kDifOk` otherwise. */ OT_WARN_UNUSED_RESULT -dif_result_t dif_spi_device_read_flash_buffer( - dif_spi_device_handle_t *spi, - dif_spi_device_flash_buffer_type_t buffer_type, uint32_t offset, - size_t length, uint8_t *buf); +dif_result_t dif_spi_device_read_flash_payload_buffer( + dif_spi_device_handle_t *spi, uint32_t offset, size_t length, uint8_t *buf); /** * Write data to one of the memories associated with flash / passthrough modes. diff --git a/sw/device/lib/dif/dif_spi_device_unittest.cc b/sw/device/lib/dif/dif_spi_device_unittest.cc index 5dc4ce724abf5..c9c199682b112 100644 --- a/sw/device/lib/dif/dif_spi_device_unittest.cc +++ b/sw/device/lib/dif/dif_spi_device_unittest.cc @@ -163,12 +163,10 @@ TEST_F(FlashTest, NullArgs) { EXPECT_DIF_BADARG( dif_spi_device_pop_flash_address_fifo(nullptr, &uint32_arg)); EXPECT_DIF_BADARG(dif_spi_device_pop_flash_address_fifo(&spi_, nullptr)); - EXPECT_DIF_BADARG(dif_spi_device_read_flash_buffer( - nullptr, kDifSpiDeviceFlashBufferTypeSfdp, /*offset=*/0, /*length=*/1, - &uint8_arg)); - EXPECT_DIF_BADARG( - dif_spi_device_read_flash_buffer(&spi_, kDifSpiDeviceFlashBufferTypeSfdp, - /*offset=*/0, /*length=*/1, nullptr)); + EXPECT_DIF_BADARG(dif_spi_device_read_flash_payload_buffer( + nullptr, /*offset=*/0, /*length=*/1, &uint8_arg)); + EXPECT_DIF_BADARG(dif_spi_device_read_flash_payload_buffer( + &spi_, /*offset=*/0, /*length=*/1, nullptr)); EXPECT_DIF_BADARG(dif_spi_device_write_flash_buffer( nullptr, kDifSpiDeviceFlashBufferTypeSfdp, /*offset=*/0, /*length=*/1, &uint8_arg)); @@ -659,24 +657,23 @@ TEST_F(FlashTest, FifoPop) { TEST_F(FlashTest, MemoryOps) { constexpr uint32_t kSfdpOffset = 3072; - constexpr uint32_t kMailboxOffset = 2048; uint32_t buf[64]; for (uint32_t i = 0; i < (sizeof(buf) / sizeof(buf[0])); i++) { buf[i] = i; - EXPECT_WRITE32( - SPI_DEVICE_BUFFER_REG_OFFSET + kSfdpOffset + i * sizeof(uint32_t), i); + EXPECT_WRITE32(SPI_DEVICE_EGRESS_BUFFER_REG_OFFSET + kSfdpOffset + + i * sizeof(uint32_t), + i); } EXPECT_DIF_OK(dif_spi_device_write_flash_buffer( &spi_, kDifSpiDeviceFlashBufferTypeSfdp, /*offset=*/0, /*length=*/sizeof(buf), reinterpret_cast(buf))); for (uint32_t i = 4; i < (sizeof(buf) / sizeof(buf[0])); i++) { - EXPECT_READ32( - SPI_DEVICE_BUFFER_REG_OFFSET + kMailboxOffset + i * sizeof(uint32_t), - 0x1000u - i); + EXPECT_READ32(SPI_DEVICE_INGRESS_BUFFER_REG_OFFSET + i * sizeof(uint32_t), + 0x1000u - i); } - EXPECT_DIF_OK(dif_spi_device_read_flash_buffer( - &spi_, kDifSpiDeviceFlashBufferTypeMailbox, /*offset=*/16, + EXPECT_DIF_OK(dif_spi_device_read_flash_payload_buffer( + &spi_, /*offset=*/16, /*length=*/sizeof(buf) - 16, reinterpret_cast(buf))); for (uint32_t i = 4; i < (sizeof(buf) / sizeof(buf[0])); i++) { EXPECT_EQ(buf[i - 4], 0x1000u - i); diff --git a/sw/device/lib/testing/spi_device_testutils.c b/sw/device/lib/testing/spi_device_testutils.c index 6c4d3982d3fac..33279e9ff9b8b 100644 --- a/sw/device/lib/testing/spi_device_testutils.c +++ b/sw/device/lib/testing/spi_device_testutils.c @@ -21,13 +21,6 @@ status_t spi_device_testutils_configure_passthrough( }; TRY(dif_spi_device_configure(spi_device, spi_device_config)); - // Zero-init the payload memory to avoid X-triggered assertions in sim. - uint8_t zeroes[256]; - memset(zeroes, 0, sizeof(zeroes)); - TRY(dif_spi_device_write_flash_buffer(spi_device, - kDifSpiDeviceFlashBufferTypePayload, 0, - sizeof(zeroes), zeroes)); - dif_spi_device_passthrough_intercept_config_t intercept_config = { .status = upload_write_commands, .jedec_id = false, @@ -341,9 +334,8 @@ status_t spi_device_testutils_wait_for_upload(dif_spi_device_handle_t *spid, // We aren't expecting more than 256 bytes of data. return INVALID_ARGUMENT(); } - TRY(dif_spi_device_read_flash_buffer(spid, - kDifSpiDeviceFlashBufferTypePayload, - start, info->data_len, info->data)); + TRY(dif_spi_device_read_flash_payload_buffer(spid, start, info->data_len, + info->data)); } // Finished: ack the IRQ. diff --git a/sw/device/silicon_creator/lib/drivers/spi_device.c b/sw/device/silicon_creator/lib/drivers/spi_device.c index 05f081c9ad13d..a6d4db2432cfc 100644 --- a/sw/device/silicon_creator/lib/drivers/spi_device.c +++ b/sw/device/silicon_creator/lib/drivers/spi_device.c @@ -23,11 +23,11 @@ enum { * Start address of the SFDP space in spi_device buffer. */ kSfdpAreaStartAddr = - kBase + SPI_DEVICE_BUFFER_REG_OFFSET + kSpiDeviceSfdpAreaOffset, + kBase + SPI_DEVICE_EGRESS_BUFFER_REG_OFFSET + kSpiDeviceSfdpAreaOffset, /** * End address (exclusive) of the SFDP space in spi_device buffer. */ - kSfdpAreaEndAddr = kBase + SPI_DEVICE_BUFFER_REG_OFFSET + + kSfdpAreaEndAddr = kBase + SPI_DEVICE_EGRESS_BUFFER_REG_OFFSET + kSpiDeviceSfdpAreaOffset + kSpiDeviceSfdpAreaNumBytes, /** * Flash data partition size in bits. @@ -544,14 +544,6 @@ void spi_device_init(void) { abs_mmio_write32(dest, UINT32_MAX); } - // Reset the payload buffer to prevent access faults when reading beyond - // current payload depth (see #11782). - for (size_t i = 0; i < kSpiDevicePayloadAreaNumBytes; i += sizeof(uint32_t)) { - abs_mmio_write32( - kBase + SPI_DEVICE_BUFFER_REG_OFFSET + kSpiDevicePayloadAreaOffset + i, - 0); - } - // Reset status register abs_mmio_write32(kBase + SPI_DEVICE_FLASH_STATUS_REG_OFFSET, 0); @@ -651,8 +643,8 @@ rom_error_t spi_device_cmd_get(spi_device_cmd_t *cmd) { bitfield_field32_read(reg, SPI_DEVICE_UPLOAD_STATUS2_PAYLOAD_DEPTH_FIELD); // `payload_byte_count` can be at most `kSpiDevicePayloadAreaNumBytes`. HARDENED_CHECK_LE(cmd->payload_byte_count, kSpiDevicePayloadAreaNumBytes); - uint32_t src = - kBase + SPI_DEVICE_BUFFER_REG_OFFSET + kSpiDevicePayloadAreaOffset; + uint32_t src = kBase + SPI_DEVICE_INGRESS_BUFFER_REG_OFFSET + + kSpiDevicePayloadAreaOffset; char *dest = (char *)&cmd->payload; for (size_t i = 0; i < cmd->payload_byte_count; i += sizeof(uint32_t)) { write_32(abs_mmio_read32(src + i), dest + i); diff --git a/sw/device/silicon_creator/lib/drivers/spi_device.h b/sw/device/silicon_creator/lib/drivers/spi_device.h index 690ab9a0d7423..3b1d6fb2439fc 100644 --- a/sw/device/silicon_creator/lib/drivers/spi_device.h +++ b/sw/device/silicon_creator/lib/drivers/spi_device.h @@ -60,25 +60,25 @@ enum { // TODO(#11740): Auto-generated macros for HW constants. enum { /** - * Size of the SFDP area in spi_device buffer in bytes. + * Size of the SFDP area in spi_device egress buffer in bytes. * * spi_device provides 256 bytes for the SFDP table. */ kSpiDeviceSfdpAreaNumBytes = 256, /** - * Offset of the SFDP area in spi_device buffer. + * Offset of the SFDP area in spi_device egress buffer. */ kSpiDeviceSfdpAreaOffset = 0xc00, /** - * Offset of the payload area in spi_device buffer. + * Offset of the payload area in spi_device ingress buffer. */ - kSpiDevicePayloadAreaOffset = 0xd00, + kSpiDevicePayloadAreaOffset = 0x0, /** - * Size of the payload area in spi_device buffer in bytes. + * Size of the payload area in spi_device ingress buffer in bytes. */ kSpiDevicePayloadAreaNumBytes = 256, /** - * Size of the payload area in spi_device buffer in words. + * Size of the payload area in spi_device ingress buffer in words. */ kSpiDevicePayloadAreaNumWords = kSpiDevicePayloadAreaNumBytes / sizeof(uint32_t), diff --git a/sw/device/silicon_creator/lib/drivers/spi_device_unittest.cc b/sw/device/silicon_creator/lib/drivers/spi_device_unittest.cc index 43093458aafa3..cbfb470bec72d 100644 --- a/sw/device/silicon_creator/lib/drivers/spi_device_unittest.cc +++ b/sw/device/silicon_creator/lib/drivers/spi_device_unittest.cc @@ -75,18 +75,12 @@ TEST_F(InitTest, Init) { std::memcpy(sfdp_buffer.data(), &kSpiDeviceSfdpTable, sizeof(kSpiDeviceSfdpTable)); uint32_t offset = - base_ + SPI_DEVICE_BUFFER_REG_OFFSET + kSpiDeviceSfdpAreaOffset; + base_ + SPI_DEVICE_EGRESS_BUFFER_REG_OFFSET + kSpiDeviceSfdpAreaOffset; for (size_t i = 0; i < sfdp_buffer.size(); ++i) { EXPECT_ABS_WRITE32(offset, sfdp_buffer[i]); offset += sizeof(uint32_t); } - offset = base_ + SPI_DEVICE_BUFFER_REG_OFFSET + kSpiDevicePayloadAreaOffset; - for (size_t i = 0; i < kSpiDevicePayloadAreaNumWords; ++i) { - EXPECT_ABS_WRITE32(offset, 0); - offset += sizeof(uint32_t); - } - EXPECT_ABS_WRITE32(base_ + SPI_DEVICE_FLASH_STATUS_REG_OFFSET, 0); EXPECT_ABS_WRITE32( @@ -233,8 +227,8 @@ TEST_P(CmdGetTest, CmdGet) { EXPECT_ABS_READ32(base_ + SPI_DEVICE_UPLOAD_STATUS2_REG_OFFSET, {{SPI_DEVICE_UPLOAD_STATUS2_PAYLOAD_DEPTH_OFFSET, GetParam().payload.size()}}); - uint32_t offset = - base_ + SPI_DEVICE_BUFFER_REG_OFFSET + kSpiDevicePayloadAreaOffset; + uint32_t offset = base_ + SPI_DEVICE_INGRESS_BUFFER_REG_OFFSET + + kSpiDevicePayloadAreaOffset; for (size_t i = 0; i < GetParam().payload.size(); i += sizeof(uint32_t)) { EXPECT_ABS_READ32(offset + i, payload_area[i / sizeof(uint32_t)]); } diff --git a/sw/device/tests/sim_dv/spi_passthrough_test.c b/sw/device/tests/sim_dv/spi_passthrough_test.c index 1768f8ea1dc0b..b47ae730e0a69 100644 --- a/sw/device/tests/sim_dv/spi_passthrough_test.c +++ b/sw/device/tests/sim_dv/spi_passthrough_test.c @@ -175,9 +175,8 @@ void handle_write_status(uint32_t status, uint8_t offset, uint8_t opcode) { CHECK_DIF_OK(dif_spi_device_get_flash_payload_fifo_occupancy( &spi_device, &occupancy, &start_offset)); CHECK(occupancy == 1); - CHECK_DIF_OK(dif_spi_device_read_flash_buffer( - &spi_device, kDifSpiDeviceFlashBufferTypePayload, start_offset, occupancy, - &payload)); + CHECK_DIF_OK(dif_spi_device_read_flash_payload_buffer( + &spi_device, start_offset, occupancy, &payload)); status &= (0xffu << offset); status |= ((uint32_t)(payload) << offset); @@ -259,9 +258,8 @@ void handle_page_program(void) { &spi_device, &payload_occupancy, &start_offset)); CHECK(start_offset == 0); CHECK(payload_occupancy <= sizeof(payload)); - CHECK_DIF_OK(dif_spi_device_read_flash_buffer( - &spi_device, kDifSpiDeviceFlashBufferTypePayload, start_offset, - payload_occupancy, payload)); + CHECK_DIF_OK(dif_spi_device_read_flash_payload_buffer( + &spi_device, start_offset, payload_occupancy, payload)); dif_toggle_t addr4b_enabled; CHECK_DIF_OK( From 13fc2e2bd2566163fc3742af4baeb1f45e169228 Mon Sep 17 00:00:00 2001 From: Alexander Williams Date: Mon, 22 Jan 2024 16:57:18 -0800 Subject: [PATCH 7/7] [spi_device] Add params for DPRAM offsets Add params for the DPRAM offsets so they are available in C headers for software. Signed-off-by: Alexander Williams --- hw/ip/spi_device/data/spi_device.hjson | 72 ++++++++++++++++++++++ hw/ip/spi_device/rtl/spi_device_pkg.sv | 31 ++++++++-- hw/ip/spi_device/rtl/spi_device_reg_pkg.sv | 12 ++++ sw/device/lib/dif/dif_spi_device.c | 25 +++++--- 4 files changed, 126 insertions(+), 14 deletions(-) diff --git a/hw/ip/spi_device/data/spi_device.hjson b/hw/ip/spi_device/data/spi_device.hjson index 32546d699bfd8..06d1bf0129242 100644 --- a/hw/ip/spi_device/data/spi_device.hjson +++ b/hw/ip/spi_device/data/spi_device.hjson @@ -139,6 +139,78 @@ default: "96" local: "true" } + { name: "SramReadBufferOffset" + desc: "Sram eFlash read buffer offset (from egress buffer start). Word size is 32bit width." + type: "int unsigned" + default: "0" + local: "true" + } + { name: "SramReadBufferDepth" + desc: "Sram eFlash read buffer entries. Word size is 32bit width." + type: "int unsigned" + default: "512" + local: "true" + } + { name: "SramMailboxOffset" + desc: "Sram mailbox buffer offset (from egress buffer start). Word size is 32bit width." + type: "int unsigned" + default: "512" + local: "true" + } + { name: "SramMailboxDepth" + desc: "Sram mailbox entries. Word size is 32bit width." + type: "int unsigned" + default: "256" + local: "true" + } + { name: "SramSfdpOffset" + desc: "Sram SFDP buffer offset (from egress buffer start). Word size is 32bit width." + type: "int unsigned" + default: "768" + local: "true" + } + { name: "SramSfdpDepth" + desc: "Sram SFDP entries. Word size is 32bit width." + type: "int unsigned" + default: "64" + local: "true" + } + { name: "SramPayloadOffset" + desc: "Sram payload FIFO offset (from ingress buffer start). Word size is 32bit width." + type: "int unsigned" + default: "0" + local: "true" + } + { name: "SramPayloadDepth" + desc: "Sram payload FIFO entries. Word size is 32bit width." + type: "int unsigned" + default: "64" + local: "true" + } + { name: "SramCmdFifoOffset" + desc: "Sram command FIFO offset (from ingress buffer start). Word size is 32bit width." + type: "int unsigned" + default: "64" + local: "true" + } + { name: "SramCmdFifoDepth" + desc: "Sram command FIFO entries. Word size is 32bit width." + type: "int unsigned" + default: "16" + local: "true" + } + { name: "SramAddrFifoOffset" + desc: "Sram address FIFO offset (from ingress buffer start). Word size is 32bit width." + type: "int unsigned" + default: "80" + local: "true" + } + { name: "SramAddrFifoDepth" + desc: "Sram address FIFO entries. Word size is 32bit width." + type: "int unsigned" + default: "16" + local: "true" + } { name: "NumCmdInfo" desc: "Define the number of Command Info slots." type: "int unsigned" diff --git a/hw/ip/spi_device/rtl/spi_device_pkg.sv b/hw/ip/spi_device/rtl/spi_device_pkg.sv index 13bd08fec403f..c36be32e39573 100644 --- a/hw/ip/spi_device/rtl/spi_device_pkg.sv +++ b/hw/ip/spi_device/rtl/spi_device_pkg.sv @@ -5,6 +5,8 @@ // Serial Peripheral Interface (SPI) Device module. // +`include "prim_assert.sv" + package spi_device_pkg; // Passthrough Inter-module signals @@ -397,22 +399,27 @@ package spi_device_pkg; parameter int unsigned SramOffsetW = $clog2(SramStrbW); // Msg region is used for read cmd in Flash and Passthrough region - parameter int unsigned SramMsgDepth = 512; // 2kB + parameter int unsigned SramMsgDepth = spi_device_reg_pkg::SramReadBufferDepth; parameter int unsigned SramMsgBytes = SramMsgDepth * SramStrbW; // Address Width of a Buffer in bytes parameter int unsigned SramBufferBytes = SramMsgBytes / 2; // Double Buffer parameter int unsigned SramBufferAw = $clog2(SramBufferBytes); - parameter int unsigned SramMailboxDepth = 256; // 1kB for mailbox + import spi_device_reg_pkg::SramMailboxDepth; + export spi_device_reg_pkg::SramMailboxDepth; // SPI Flash Discoverable Parameter is for host to get the device // information. It is a separate Command. The device provides a region in // DPSRAM for SW. The size is 256B - parameter int unsigned SramSfdpDepth = 64; - parameter int unsigned SramPayloadDepth = 64; // 256B for Program - parameter int unsigned SramCmdFifoDepth = 16; // 16 x 1B for Cmd - parameter int unsigned SramAddrFifoDepth = 16; // 16 x 4B for Addr + import spi_device_reg_pkg::SramSfdpDepth; + export spi_device_reg_pkg::SramSfdpDepth; + import spi_device_reg_pkg::SramPayloadDepth; + export spi_device_reg_pkg::SramPayloadDepth; + import spi_device_reg_pkg::SramCmdFifoDepth; + export spi_device_reg_pkg::SramCmdFifoDepth; + import spi_device_reg_pkg::SramAddrFifoDepth; + export spi_device_reg_pkg::SramAddrFifoDepth; parameter int unsigned SramTotalDepth = SramMsgDepth + SramMailboxDepth + SramSfdpDepth + SramPayloadDepth + SramCmdFifoDepth + SramAddrFifoDepth ; @@ -478,23 +485,35 @@ package spi_device_pkg; sram_addr_t'(SramEgressByteOffset[$clog2(SramDw / 8) +: SramAw]); parameter sram_addr_t SramReadBufferIdx = SramEgressIdx; parameter sram_addr_t SramReadBufferSize = sram_addr_t'(SramMsgDepth); + `ASSERT_STATIC_IN_PACKAGE(CheckReadBufferOffset, + SramReadBufferIdx == spi_device_reg_pkg::SramReadBufferOffset) parameter sram_addr_t SramMailboxIdx = SramReadBufferIdx + SramReadBufferSize; parameter sram_addr_t SramMailboxSize = sram_addr_t'(SramMailboxDepth); + `ASSERT_STATIC_IN_PACKAGE(CheckMailboxOffset, + SramMailboxIdx == spi_device_reg_pkg::SramMailboxOffset) parameter sram_addr_t SramSfdpIdx = SramMailboxIdx + SramMailboxSize; parameter sram_addr_t SramSfdpSize = sram_addr_t'(SramSfdpDepth); + `ASSERT_STATIC_IN_PACKAGE(CheckSfdpOffset, + SramSfdpIdx == spi_device_reg_pkg::SramSfdpOffset) parameter sram_addr_t SramIngressIdx = sram_addr_t'(SramIngressByteOffset[$clog2(SramDw / 8) +: SramAw]); parameter sram_addr_t SramPayloadIdx = SramIngressIdx; parameter sram_addr_t SramPayloadSize = sram_addr_t'(SramPayloadDepth); + `ASSERT_STATIC_IN_PACKAGE(CheckPayloadOffset, + (SramPayloadIdx - SramIngressIdx) == spi_device_reg_pkg::SramPayloadOffset) parameter sram_addr_t SramCmdFifoIdx = SramPayloadIdx + SramPayloadSize; parameter sram_addr_t SramCmdFifoSize = sram_addr_t'(SramCmdFifoDepth); + `ASSERT_STATIC_IN_PACKAGE(CheckCmdFifoOffset, + (SramCmdFifoIdx - SramIngressIdx) == spi_device_reg_pkg::SramCmdFifoOffset) parameter sram_addr_t SramAddrFifoIdx = SramCmdFifoIdx + SramCmdFifoSize; parameter sram_addr_t SramAddrFifoSize = sram_addr_t'(SramAddrFifoDepth); + `ASSERT_STATIC_IN_PACKAGE(CheckAddrFifoOffset, + (SramAddrFifoIdx - SramIngressIdx) == spi_device_reg_pkg::SramAddrFifoOffset) // Max BitCount in a transaction parameter int unsigned BitLength = SramMsgDepth * 32; diff --git a/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv b/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv index d947eab8c52e8..41f9b0ba89ffa 100644 --- a/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv +++ b/hw/ip/spi_device/rtl/spi_device_reg_pkg.sv @@ -10,6 +10,18 @@ package spi_device_reg_pkg; parameter int unsigned SramDepth = 1024; parameter int unsigned SramEgressDepth = 832; parameter int unsigned SramIngressDepth = 96; + parameter int unsigned SramReadBufferOffset = 0; + parameter int unsigned SramReadBufferDepth = 512; + parameter int unsigned SramMailboxOffset = 512; + parameter int unsigned SramMailboxDepth = 256; + parameter int unsigned SramSfdpOffset = 768; + parameter int unsigned SramSfdpDepth = 64; + parameter int unsigned SramPayloadOffset = 0; + parameter int unsigned SramPayloadDepth = 64; + parameter int unsigned SramCmdFifoOffset = 64; + parameter int unsigned SramCmdFifoDepth = 16; + parameter int unsigned SramAddrFifoOffset = 80; + parameter int unsigned SramAddrFifoDepth = 16; parameter int unsigned NumCmdInfo = 24; parameter int unsigned NumLocality = 5; parameter int unsigned TpmWrFifoPtrW = 7; diff --git a/sw/device/lib/dif/dif_spi_device.c b/sw/device/lib/dif/dif_spi_device.c index 5e1b7c134b401..a7c80d9f15c6c 100644 --- a/sw/device/lib/dif/dif_spi_device.c +++ b/sw/device/lib/dif/dif_spi_device.c @@ -13,16 +13,25 @@ #define DIF_SPI_DEVICE_TPM_FIFO_DEPTH 16 enum { kDifSpiDeviceFlashStatusWelBit = 1 }; -enum { kDifSpiDeviceEFlashLen = 2048 }; -enum { kDifSpiDeviceMailboxLen = 1024 }; -enum { kDifSpiDeviceSfdpLen = 256 }; -enum { kDifSpiDevicePayloadLen = 256 }; +enum { + kDifSpiDeviceEFlashLen = + SPI_DEVICE_PARAM_SRAM_READ_BUFFER_DEPTH * sizeof(uint32_t), + kDifSpiDeviceMailboxLen = + SPI_DEVICE_PARAM_SRAM_MAILBOX_DEPTH * sizeof(uint32_t), + kDifSpiDeviceSfdpLen = SPI_DEVICE_PARAM_SRAM_SFDP_DEPTH * sizeof(uint32_t), + kDifSpiDevicePayloadLen = + SPI_DEVICE_PARAM_SRAM_PAYLOAD_DEPTH * sizeof(uint32_t), +}; enum { - kDifSpiDeviceEFlashOffset = 0, - kDifSpiDeviceMailboxOffset = 2048, - kDifSpiDeviceSfdpOffset = 3072, - kDifSpiDevicePayloadOffset = 0, + kDifSpiDeviceEFlashOffset = + SPI_DEVICE_PARAM_SRAM_READ_BUFFER_OFFSET * sizeof(uint32_t), + kDifSpiDeviceMailboxOffset = + SPI_DEVICE_PARAM_SRAM_MAILBOX_OFFSET * sizeof(uint32_t), + kDifSpiDeviceSfdpOffset = + SPI_DEVICE_PARAM_SRAM_SFDP_OFFSET * sizeof(uint32_t), + kDifSpiDevicePayloadOffset = + SPI_DEVICE_PARAM_SRAM_PAYLOAD_OFFSET * sizeof(uint32_t), }; /**