Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[racl] Follow-up fixes to TLUL adapter and reg_top #26056

Merged
merged 4 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hw/ip/i2c/rtl/i2c.sv
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module i2c
parameter logic [NumAlerts-1:0] AlertAsyncOn = {NumAlerts{1'b1}},
parameter int unsigned InputDelayCycles = 0,
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = 1'b1,
parameter bit RaclErrorRsp = EnableRacl,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm convinced that this is cleaner code, so it looks like the right thing to do. But does it actually change behaviour? I think the parameter is only used to qualify racl_error_o. This, in turn, is only true if we hit a register that doesn't have a matching racl permission and every register gets a permission if EnableRacl is false.

Is that correct? Probably worth explaining in the commit message why the change is needed (either cleaning stuff up or fixing some bug that Rupert didn't see!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No there is no change in behaviour. Is proposed by Adrian, this is jus a safeguard to not send out a spurious racl error if RACL is disabled. Nothing else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I included that in the commit message.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rswarbrick If the racl_error_o generation logic is correct, such that RACL errors cannot be raised spuriously, then it has no functional impact on non-RACL designs. It's belt'n'braces.

parameter int unsigned RaclPolicySelVec[32] = '{32{0}}
) (
input clk_i,
Expand Down
6 changes: 3 additions & 3 deletions hw/ip/i2c/rtl/i2c_reg_top.sv
Original file line number Diff line number Diff line change
Expand Up @@ -3471,9 +3471,9 @@ module i2c_reg_top
end

assign addrmiss = (reg_re || reg_we) ? ~|addr_hit : 1'b0 ;
// A valid address hit but failed the RACL check
assign racl_error_o = tl_i.a_valid &
(|addr_hit) & ~(|(addr_hit & (racl_addr_hit_read | racl_addr_hit_write)));
// A valid address hit, access, but failed the RACL check
assign racl_error_o = |addr_hit & ((reg_re & ~|racl_addr_hit_read) |
(reg_we & ~|racl_addr_hit_write));
assign racl_error_log_o.racl_role = racl_role;

if (EnableRacl) begin : gen_racl_log
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/mbx/rtl/mbx.sv
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module mbx
parameter bit DoeIrqSupport = 1'b1,
parameter bit DoeAsyncMsgSupport = 1'b1,
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = 1'b1,
parameter bit RaclErrorRsp = EnableRacl,
parameter int unsigned RaclPolicySelVecSoc[4] = '{4{0}},
parameter int unsigned RaclPolicySelWinSocWDATA = 0,
parameter int unsigned RaclPolicySelWinSocRDATA = 0
Expand Down
6 changes: 3 additions & 3 deletions hw/ip/mbx/rtl/mbx_soc_reg_top.sv
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@ module mbx_soc_reg_top
end

assign addrmiss = (reg_re || reg_we) ? ~|addr_hit : 1'b0 ;
// A valid address hit but failed the RACL check
assign racl_error_o = tl_i.a_valid &
(|addr_hit) & ~(|(addr_hit & (racl_addr_hit_read | racl_addr_hit_write)));
// A valid address hit, access, but failed the RACL check
assign racl_error_o = |addr_hit & ((reg_re & ~|racl_addr_hit_read) |
(reg_we & ~|racl_addr_hit_write));
assign racl_error_log_o.racl_role = racl_role;

if (EnableRacl) begin : gen_racl_log
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/pwm/rtl/pwm.sv
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module pwm
#(
parameter logic [NumAlerts-1:0] AlertAsyncOn = {NumAlerts{1'b1}},
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = 1'b1,
parameter bit RaclErrorRsp = EnableRacl,
parameter int unsigned RaclPolicySelVec[23] = '{23{0}},
parameter int PhaseCntDw = 16,
parameter int BeatCntDw = 27
Expand Down
6 changes: 3 additions & 3 deletions hw/ip/pwm/rtl/pwm_reg_top.sv
Original file line number Diff line number Diff line change
Expand Up @@ -3164,9 +3164,9 @@ module pwm_reg_top
end

assign addrmiss = (reg_re || reg_we) ? ~|addr_hit : 1'b0 ;
// A valid address hit but failed the RACL check
assign racl_error_o = tl_i.a_valid &
(|addr_hit) & ~(|(addr_hit & (racl_addr_hit_read | racl_addr_hit_write)));
// A valid address hit, access, but failed the RACL check
assign racl_error_o = |addr_hit & ((reg_re & ~|racl_addr_hit_read) |
(reg_we & ~|racl_addr_hit_write));
assign racl_error_log_o.racl_role = racl_role;

if (EnableRacl) begin : gen_racl_log
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/spi_host/rtl/spi_host.sv
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module spi_host
parameter logic [NumAlerts-1:0] AlertAsyncOn = {NumAlerts{1'b1}},
parameter int unsigned NumCS = 1,
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = 1'b1,
parameter bit RaclErrorRsp = EnableRacl,
parameter int unsigned RaclPolicySelVec[12] = '{12{0}},
parameter int unsigned RaclPolicySelWinRXDATA = 0,
parameter int unsigned RaclPolicySelWinTXDATA = 0
Expand Down
6 changes: 3 additions & 3 deletions hw/ip/spi_host/rtl/spi_host_reg_top.sv
Original file line number Diff line number Diff line change
Expand Up @@ -1781,9 +1781,9 @@ module spi_host_reg_top
end

assign addrmiss = (reg_re || reg_we) ? ~|addr_hit : 1'b0 ;
// A valid address hit but failed the RACL check
assign racl_error_o = tl_i.a_valid &
(|addr_hit) & ~(|(addr_hit & (racl_addr_hit_read | racl_addr_hit_write)));
// A valid address hit, access, but failed the RACL check
assign racl_error_o = |addr_hit & ((reg_re & ~|racl_addr_hit_read) |
(reg_we & ~|racl_addr_hit_write));
assign racl_error_log_o.racl_role = racl_role;

if (EnableRacl) begin : gen_racl_log
Expand Down
27 changes: 15 additions & 12 deletions hw/ip/tlul/rtl/tlul_adapter_reg_racl.sv
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ module tlul_adapter_reg_racl
import tlul_pkg::*;
import prim_mubi_pkg::mubi4_t;
#(
parameter bit CmdIntgCheck = 0, // 1: Enable command integrity check
parameter bit EnableRspIntgGen = 0, // 1: Generate response integrity
parameter bit EnableDataIntgGen = 0, // 1: Generate response data integrity
parameter int RegAw = 8, // Width of register address
parameter int RegDw = 32, // Shall be matched with TL_DW
parameter int AccessLatency = 0, // 0: same cycle, 1: next cycle
parameter bit EnableRacl = 0, // 1: Enable RACL checks on access
parameter bit RaclErrorRsp = 1, // 1: Return TLUL error on RACL errors
parameter int RaclPolicySelVec = 0, // RACL policy for this reg adapter
parameter bit CmdIntgCheck = 0, // 1: Enable command integrity check
parameter bit EnableRspIntgGen = 0, // 1: Generate response integrity
parameter bit EnableDataIntgGen = 0, // 1: Generate response data integrity
parameter int RegAw = 8, // Width of register address
parameter int RegDw = 32, // Shall be matched with TL_DW
parameter int AccessLatency = 0, // 0: same cycle, 1: next cycle
parameter bit EnableRacl = 0, // 1: Enable RACL checks on access
parameter bit RaclErrorRsp = EnableRacl, // 1: Return TLUL error on RACL errors
parameter int RaclPolicySelVec = 0, // RACL policy for this reg adapter
localparam int RegBw = RegDw/8
) (
input clk_i,
Expand Down Expand Up @@ -69,11 +69,14 @@ module tlul_adapter_reg_racl
.out_o( racl_role_vec )
);

logic rd_req, racl_read_allowed, racl_write_allowed;
assign rd_req = tl_i.a_opcode == tlul_pkg::Get;
logic req, rd_req, wr_req, racl_read_allowed, racl_write_allowed;
assign req = tl_i.a_valid & tl_o.a_ready;
assign rd_req = req & (tl_i.a_opcode == tlul_pkg::Get);
assign wr_req = req & (tl_i.a_opcode == tlul_pkg::PutFullData |
tl_i.a_opcode == tlul_pkg::PutPartialData);
assign racl_read_allowed = (|(racl_policies_i[RaclPolicySelVec].read_perm & racl_role_vec));
assign racl_write_allowed = (|(racl_policies_i[RaclPolicySelVec].write_perm & racl_role_vec));
assign racl_error_o = (rd_req & ~racl_read_allowed) | (~rd_req & ~racl_write_allowed);
assign racl_error_o = (rd_req & ~racl_read_allowed) | (wr_req & ~racl_write_allowed);

tlul_request_loopback #(
.ErrorRsp(RaclErrorRsp)
Expand Down
48 changes: 26 additions & 22 deletions hw/ip/tlul/rtl/tlul_adapter_sram_racl.sv
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,26 @@ module tlul_adapter_sram_racl
import prim_mubi_pkg::mubi4_t;
#(
parameter int SramAw = 12,
parameter int SramDw = 32, // Must be multiple of the TL width
parameter int Outstanding = 1, // Only one request is accepted
parameter int SramBusBankAW = 12, // SRAM bus address width of the SRAM bank. Only used
// when DataXorAddr=1.
parameter bit ByteAccess = 1, // 1: Enables sub-word write transactions. Note that this
// results in read-modify-write operations for integrity
// re-generation if EnableDataIntgPt is set to 1.
parameter bit ErrOnWrite = 0, // 1: Writes not allowed, automatically error
parameter bit ErrOnRead = 0, // 1: Reads not allowed, automatically error
parameter bit CmdIntgCheck = 0, // 1: Enable command integrity check
parameter bit EnableRspIntgGen = 0, // 1: Generate response integrity
parameter bit EnableDataIntgGen = 0, // 1: Generate response data integrity
parameter bit EnableDataIntgPt = 0, // 1: Passthrough command/response data integrity
parameter bit SecFifoPtr = 0, // 1: Duplicated fifo pointers
parameter bit EnableReadback = 0, // 1: Readback and check written/read data.
parameter bit DataXorAddr = 0, // 1: XOR data and address for address protection
parameter bit EnableRacl = 0, // 1: Enable RACL checks on access
parameter bit RaclErrorRsp = 1, // 1: Return TLUL error on RACL errors
parameter int RaclPolicySelVec = 0, // RACL policy for this SRAM adapter
parameter int SramDw = 32, // Must be multiple of the TL width
parameter int Outstanding = 1, // Only one request is accepted
parameter int SramBusBankAW = 12, // SRAM bus address width of the SRAM bank. Only
// used when DataXorAddr=1.
parameter bit ByteAccess = 1, // 1: Enables sub-word write transactions. Note that
// this results in read-modify-write operations
// for integrity re-generation if
// EnableDataIntgPt is set to 1.
parameter bit ErrOnWrite = 0, // 1: Writes not allowed, automatically error
parameter bit ErrOnRead = 0, // 1: Reads not allowed, automatically error
parameter bit CmdIntgCheck = 0, // 1: Enable command integrity check
parameter bit EnableRspIntgGen = 0, // 1: Generate response integrity
parameter bit EnableDataIntgGen = 0, // 1: Generate response data integrity
parameter bit EnableDataIntgPt = 0, // 1: Passthrough command/response data integrity
parameter bit SecFifoPtr = 0, // 1: Duplicated fifo pointers
parameter bit EnableReadback = 0, // 1: Readback and check written/read data.
parameter bit DataXorAddr = 0, // 1: XOR data and address for address protection
parameter bit EnableRacl = 0, // 1: Enable RACL checks on access
parameter bit RaclErrorRsp = EnableRacl, // 1: Return TLUL error on RACL errors
parameter int RaclPolicySelVec = 0, // RACL policy for this SRAM adapter
localparam int WidthMult = SramDw / top_pkg::TL_DW,
localparam int IntgWidth = tlul_pkg::DataIntgWidth * WidthMult,
localparam int DataOutW = EnableDataIntgPt ? SramDw + IntgWidth : SramDw
Expand Down Expand Up @@ -95,11 +96,14 @@ module tlul_adapter_sram_racl
.out_o( racl_role_vec )
);

logic rd_req, racl_read_allowed, racl_write_allowed;
assign rd_req = tl_i.a_opcode == tlul_pkg::Get;
logic req, rd_req, wr_req, racl_read_allowed, racl_write_allowed;
assign req = tl_i.a_valid & tl_o.a_ready;
assign rd_req = req & (tl_i.a_opcode == tlul_pkg::Get);
assign wr_req = req & (tl_i.a_opcode == tlul_pkg::PutFullData |
tl_i.a_opcode == tlul_pkg::PutPartialData);
assign racl_read_allowed = (|(racl_policies_i[RaclPolicySelVec].read_perm & racl_role_vec));
assign racl_write_allowed = (|(racl_policies_i[RaclPolicySelVec].write_perm & racl_role_vec));
assign racl_error_o = (rd_req & ~racl_read_allowed) | (~rd_req & ~racl_write_allowed);
assign racl_error_o = (rd_req & ~racl_read_allowed) | (wr_req & ~racl_write_allowed);

tlul_request_loopback #(
.ErrorRsp(RaclErrorRsp)
Expand Down
2 changes: 1 addition & 1 deletion hw/ip/uart/rtl/uart.sv
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module uart
#(
parameter logic [NumAlerts-1:0] AlertAsyncOn = {NumAlerts{1'b1}},
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = 1'b1,
parameter bit RaclErrorRsp = EnableRacl,
parameter int unsigned RaclPolicySelVec[13] = '{13{0}}
) (
input clk_i,
Expand Down
6 changes: 3 additions & 3 deletions hw/ip/uart/rtl/uart_reg_top.sv
Original file line number Diff line number Diff line change
Expand Up @@ -1628,9 +1628,9 @@ module uart_reg_top
end

assign addrmiss = (reg_re || reg_we) ? ~|addr_hit : 1'b0 ;
// A valid address hit but failed the RACL check
assign racl_error_o = tl_i.a_valid &
(|addr_hit) & ~(|(addr_hit & (racl_addr_hit_read | racl_addr_hit_write)));
// A valid address hit, access, but failed the RACL check
assign racl_error_o = |addr_hit & ((reg_re & ~|racl_addr_hit_read) |
(reg_we & ~|racl_addr_hit_write));
assign racl_error_log_o.racl_role = racl_role;

if (EnableRacl) begin : gen_racl_log
Expand Down
3 changes: 2 additions & 1 deletion hw/ip_templates/ac_range_check/rtl/ac_range_check.sv.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ module ${module_instance_name}
import ${module_instance_name}_reg_pkg::*;
#(
parameter logic [NumAlerts-1:0] AlertAsyncOn = {NumAlerts{1'b1}},
parameter bit RaclErrorRsp = 1'b1,
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = EnableRacl,
parameter int unsigned RaclPolicySelVec[${3 + 5*num_ranges}] = '{${3 + 5*num_ranges}{0}}
) (
input logic clk_i,
Expand Down
2 changes: 1 addition & 1 deletion hw/ip_templates/alert_handler/rtl/alert_handler.sv.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module ${module_instance_name}
#(
% if racl_support:
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = 1'b1,
parameter bit RaclErrorRsp = EnableRacl,
<%
num_regs = 6 + 4 * n_alerts + 4 * 7 + n_classes * 14
%>\
Expand Down
4 changes: 2 additions & 2 deletions hw/ip_templates/rv_plic/rtl/rv_plic.sv.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ module ${module_instance_name} import ${module_instance_name}_reg_pkg::*; #(
// and routing the source clocks / resets to the PLIC).
parameter logic [NumSrc-1:0] LevelEdgeTrig = '0, // 0: level, 1: edge
% if racl_support:
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = 1'b1,
parameter bit EnableRacl = 1'b0,
parameter bit RaclErrorRsp = EnableRacl,
<%
from math import ceil
num_regs = src + ceil(src / 32) + target * ceil(src / 32) + 3 * target + 1
Expand Down
6 changes: 3 additions & 3 deletions util/reggen/reg_top.sv.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -730,9 +730,9 @@ ${finst_gen(sr, field, finst_name, fsig_name, fidx)}

assign addrmiss = (reg_re || reg_we) ? ~|addr_hit : 1'b0 ;
% if racl_support:
// A valid address hit but failed the RACL check
assign racl_error_o = tl_i.a_valid &
(|addr_hit) & ~(|(addr_hit & (racl_addr_hit_read | racl_addr_hit_write)));
Razer6 marked this conversation as resolved.
Show resolved Hide resolved
// A valid address hit, access, but failed the RACL check
assign racl_error_o = |addr_hit & ((reg_re & ~|racl_addr_hit_read) |
(reg_we & ~|racl_addr_hit_write));
assign racl_error_log_o.racl_role = racl_role;

if (EnableRacl) begin : gen_racl_log
Expand Down
Loading