[tlul,rtl] Correct the calculation of data_intg in tlul_adapter_sram

This is the value that gets supplied as a user field that provides
data integrity. If we are responding with an error response, we need
to make sure we send integrity bits that correspond.

The logic that was previously here detected this condition with

  (vld_rd_rsp && reqfifo_rdata.error)

but that is wrong because it shouldn't depend on vld_rd_rsp. Imagine
we send a TL write with a TL error. When we read the response, the
d_error flag will be high (because u_reqfifo contains the faulty TL
write) and d_data will be error_blanking_data. But the integrity bits
will be SecdedInv3932ZeroEcc because vld_rd_rsp is false (we haven't
seen a TL read at all!)

This is also possible to trigger by using only reads. Suppose we send
an TL read with a TL error and then, a few cycles later, read the
TL response.

When we read the response, the d_error flag will again be
high (because u_reqfifo contains the faulty TL read). Again, d_data
will (correctly) be error_blanking_data. Again, we should be using
error_blanking_integ for error bits but we actually use
SecdedInv3932ZeroEcc.

Dropping the vld_rd_rsp term will fix the behaviour in both cases.

So it remains in sync with the RTL, we also drop a conditional
coverage exclusion for rom_ctrl. Tracking down how it was actually
possible to see this happen led us to the design change.

Signed-off-by: Rupert Swarbrick <[email protected]>

hw/ip/rom_ctrl/dv/cov/rom_ctrl_cov_excl.el

@@ -98,15 +98,6 @@ Condition 21 "3623514242" "(d_valid & rspfifo_rvalid & (reqfifo_rdata.op == OpRe
 // popped again before anything appears in the response fifo.
 Condition 24 "1059982851" "(vld_rd_rsp & ((~d_error))) 1 -1" (2 "10")
 
-// The reason for condition 24 explains why we can't get (vld_rd_rsp && reqfifo_rdata.error) = 1 as
-// if vld_rd_rsp = 1, this means that rspfifo_rvalid = 1. But when we have reqfifo_rdata.error, the
-// request from the req_fifo gets popped again before becoming visible to response fifo. This also
-// explains why we can't expect 1 and 11 for condition 25 and 26 respectively.
-Condition 25 "2807788926" "((vld_rd_rsp && reqfifo_rdata.error) ? error_blanking_integ :
-                            (vld_rd_rsp ? rspfifo_rdata.data_intg :
-                            prim_secded_pkg::SecdedInv3932ZeroEcc)) 1 -1" (2 "1")
-Condition 26 "561780173" "(vld_rd_rsp && reqfifo_rdata.error) 1 -1" (3 "11")
-
 // We cannot see d_valid = 0 and d_error = 1. For d_error to be true, we need reqfifo_rvalid to be
 // true. But then the only way for d_valid to be false is if rspfifo_rvalid is false (and
 // reqfifo_rdata.op is OpRead).

hw/ip/tlul/rtl/tlul_adapter_sram.sv

@@ -345,9 +345,9 @@ module tlul_adapter_sram
   // If this a write response with data fields set to 0, we have to set all ECC bits correctly
   // since we are using an inverted Hsiao code.
   logic [DataIntgWidth-1:0] data_intg;
-  assign data_intg = (vld_rd_rsp && reqfifo_rdata.error) ? error_blanking_integ    : // TL-UL error
-                     (vld_rd_rsp)                        ? rspfifo_rdata.data_intg : // valid read
-                     prim_secded_pkg::SecdedInv3932ZeroEcc;                          // valid write
+  assign data_intg = (reqfifo_rdata.error) ? error_blanking_integ    : // TL-UL error
+                     (vld_rd_rsp)          ? rspfifo_rdata.data_intg : // valid read
+                     prim_secded_pkg::SecdedInv3932ZeroEcc;            // valid write
 
   // When an error is seen on an incoming transaction it gets an immediate response without
   // performing an SRAM request. It may be the transaction receives a ready the first cycle it is