[prim, rom_ctrl] Remove S&P layer from data scrambling

As elaborated on #20788, the S&P layer is disabled in the SRAM scrambling
devices in order to improve error detection guarantees, interactions with
ECC and timing.

In order to minimize changes and keep the implementation around in case
it is needed for byte parity at some point, we just set the NumDiffRounds
parameter to zero for the modules that leverage prim_ram_1p_scr.

In case of rom_ctrl, the functionality is removed entirely.

Signed-off-by: Michael Schaffner <[email protected]>

hw/dv/sv/mem_bkdr_util/mem_bkdr_util__rom.sv

@@ -59,7 +59,6 @@ virtual function bit [38:0] rom_encrypt_read32(bit [bus_params_pkg::BUS_AW-1:0]
     zero_key[i] = '0;
   end
 
-  data_arr = sram_scrambler_pkg::sp_decrypt(data_arr, 39, zero_key);
   for (int i = 0; i < 39; i++) begin
     data[i] = data_arr[i] ^ keystream[i];
   end

hw/dv/sv/mem_bkdr_util/sram_scrambler_pkg.sv

@@ -237,10 +237,10 @@ package sram_scrambler_pkg;
   // SRAM data encryption is more involved, we need to run 2 rounds of PRINCE on the nonce and key
   // and then XOR the result with the data.
   //
-  // After that, the XORed data neeeds to them be passed through the S&P network one byte at a time.
+  // Optionally, the XORed data can be passed through the S&P network.
   function automatic state_t encrypt_sram_data(logic data[], int data_width, int sp_width,
                                                logic addr[], int addr_width,
-                                               logic key[], logic nonce[]);
+                                               logic key[], logic nonce[], bit use_sp_layer = 0);
     logic keystream[] = new[SRAM_BLOCK_WIDTH];
     logic data_enc[] = new[data_width];
     logic byte_to_enc[] = new[8];
@@ -262,31 +262,33 @@ package sram_scrambler_pkg;
       data_enc[i] = data[i] ^ keystream[i % ks_width];
     end
 
-    if (data_width == sp_width) begin
-      // pass the entire word through the subst/perm network at once (the next cases would give the
-      // same results too, but this should be a bit more efficient)
-      data_enc = sp_encrypt(data_enc, data_width, zero_key);
-    end else if (sp_width == 8) begin
-      // pass each byte of the encoded result through the subst/perm network (special case of the
-      // general code below)
-      for (int i = 0; i < data_width / 8; i++) begin
-        byte_to_enc = data_enc[i*8 +: 8];
-        enc_byte = sp_encrypt(byte_to_enc, 8, zero_key);
-        data_enc[i*8 +: 8] = enc_byte;
-      end
-    end else begin
-      // divide the word into sp_width chunks to pass it through the subst/perm network
-      for (int chunk_lsb = 0; chunk_lsb < data_width; chunk_lsb += sp_width) begin
-        int bits_remaining = data_width - chunk_lsb;
-        int chunk_width = (bits_remaining < sp_width) ? bits_remaining : sp_width;
-        logic chunk[] = new[chunk_width];
-
-        for (int j = 0; j < chunk_width; j++) begin
-          chunk[j] = data_enc[chunk_lsb + j];
+    if (use_sp_layer) begin
+      if (data_width == sp_width) begin
+        // pass the entire word through the subst/perm network at once (the next cases would give the
+        // same results too, but this should be a bit more efficient)
+        data_enc = sp_encrypt(data_enc, data_width, zero_key);
+      end else if (sp_width == 8) begin
+        // pass each byte of the encoded result through the subst/perm network (special case of the
+        // general code below)
+        for (int i = 0; i < data_width / 8; i++) begin
+          byte_to_enc = data_enc[i*8 +: 8];
+          enc_byte = sp_encrypt(byte_to_enc, 8, zero_key);
+          data_enc[i*8 +: 8] = enc_byte;
         end
-        chunk = sp_encrypt(chunk, chunk_width, zero_key);
-        for (int j = 0; j < chunk_width; j++) begin
-          data_enc[chunk_lsb + j] = chunk[j];
+      end else begin
+        // divide the word into sp_width chunks to pass it through the subst/perm network
+        for (int chunk_lsb = 0; chunk_lsb < data_width; chunk_lsb += sp_width) begin
+          int bits_remaining = data_width - chunk_lsb;
+          int chunk_width = (bits_remaining < sp_width) ? bits_remaining : sp_width;
+          logic chunk[] = new[chunk_width];
+
+          for (int j = 0; j < chunk_width; j++) begin
+            chunk[j] = data_enc[chunk_lsb + j];
+          end
+          chunk = sp_encrypt(chunk, chunk_width, zero_key);
+          for (int j = 0; j < chunk_width; j++) begin
+            data_enc[chunk_lsb + j] = chunk[j];
+          end
         end
       end
     end
@@ -296,7 +298,7 @@ package sram_scrambler_pkg;
 
   function automatic state_t decrypt_sram_data(logic data[], int data_width, int sp_width,
                                                logic addr[], int addr_width,
-                                               logic key[], logic nonce[]);
+                                               logic key[], logic nonce[], bit use_sp_layer = 0);
     logic keystream[] = new[SRAM_BLOCK_WIDTH];
     logic data_dec[] = new[data_width];
     logic byte_to_dec[] = new[8];
@@ -312,31 +314,33 @@ package sram_scrambler_pkg;
     // Generate the keystream
     keystream = gen_keystream(addr, addr_width, key, nonce);
 
-    if (data_width == sp_width) begin
-      // pass the entire word through the subst/perm network at once (the next cases would give the
-      // same results too, but this should be a bit more efficient)
-      data_dec = sp_decrypt(data, data_width, zero_key);
-    end else if (sp_width == 8) begin
-      // pass each byte of the data through the subst/perm network (special case of the general code
-      // below)
-      for (int i = 0; i < data_width / 8; i++) begin
-        byte_to_dec = data[i*8 +: 8];
-        dec_byte = sp_decrypt(byte_to_dec, 8, zero_key);
-        data_dec[i*8 +: 8] = dec_byte;
-      end
-    end else begin
-      // divide the word into sp_width chunks to pass it through the subst/perm network
-      for (int chunk_lsb = 0; chunk_lsb < data_width; chunk_lsb += sp_width) begin
-        int bits_remaining = data_width - chunk_lsb;
-        int chunk_width = (bits_remaining < sp_width) ? bits_remaining : sp_width;
-        logic chunk[] = new[chunk_width];
-
-        for (int j = 0; j < chunk_width; j++) begin
-          chunk[j] = data[chunk_lsb + j];
+    if (use_sp_layer) begin
+      if (data_width == sp_width) begin
+        // pass the entire word through the subst/perm network at once (the next cases would give the
+        // same results too, but this should be a bit more efficient)
+        data_dec = sp_decrypt(data, data_width, zero_key);
+      end else if (sp_width == 8) begin
+        // pass each byte of the data through the subst/perm network (special case of the general code
+        // below)
+        for (int i = 0; i < data_width / 8; i++) begin
+          byte_to_dec = data[i*8 +: 8];
+          dec_byte = sp_decrypt(byte_to_dec, 8, zero_key);
+          data_dec[i*8 +: 8] = dec_byte;
         end
-        chunk = sp_decrypt(chunk, chunk_width, zero_key);
-        for (int j = 0; j < chunk_width; j++) begin
-          data_dec[chunk_lsb + j] = chunk[j];
+      end else begin
+        // divide the word into sp_width chunks to pass it through the subst/perm network
+        for (int chunk_lsb = 0; chunk_lsb < data_width; chunk_lsb += sp_width) begin
+          int bits_remaining = data_width - chunk_lsb;
+          int chunk_width = (bits_remaining < sp_width) ? bits_remaining : sp_width;
+          logic chunk[] = new[chunk_width];
+
+          for (int j = 0; j < chunk_width; j++) begin
+            chunk[j] = data[chunk_lsb + j];
+          end
+          chunk = sp_decrypt(chunk, chunk_width, zero_key);
+          for (int j = 0; j < chunk_width; j++) begin
+            data_dec[chunk_lsb + j] = chunk[j];
+          end
         end
       end
     end

hw/ip/otbn/rtl/otbn.sv

@@ -332,8 +332,7 @@ module otbn
     .Width          (39),
     .Depth          (ImemSizeWords),
     .DataBitsPerMask(39),
-    .EnableParity   (0),
-    .DiffWidth      (39)
+    .EnableParity   (0)
   ) u_imem (
     .clk_i,
     .rst_ni(rst_n),
@@ -535,7 +534,6 @@ module otbn
     .Depth             (DmemSizeWords),
     .DataBitsPerMask   (39),
     .EnableParity      (0),
-    .DiffWidth         (39),
     .ReplicateKeyStream(1)
   ) u_dmem (
     .clk_i,

hw/ip/prim/rtl/prim_ram_1p_scr.sv

@@ -35,7 +35,10 @@ module prim_ram_1p_scr import prim_ram_1p_pkg::*; #(
   // Number of PRINCE half rounds, can be [1..5]
   parameter  int NumPrinceRoundsHalf = 2,
   // Number of extra diffusion rounds. Setting this to 0 to disable diffusion.
-  parameter  int NumDiffRounds       = 2,
+  // NOTE: this setting is disabled by default, since it can interact adversely when using this
+  // primitive with end-to-end ECC integrity. Only enable this if you know what you are doing. See
+  // also #20788 for more context.
+  parameter  int NumDiffRounds       = 0,
   // This parameter governs the block-width of additional diffusion layers.
   // For intra-byte diffusion, set this parameter to 8.
   parameter  int DiffWidth           = DataBitsPerMask,

hw/ip/rom_ctrl/doc/theory_of_operation.md

@@ -132,7 +132,7 @@ To avoid attacks propagating into the rest of the system, we take the following
 Parameter                   | Default (Max)         | Top Earlgrey | Description
 ----------------------------|-----------------------|--------------|---------------
 `RndCnstRomKey`             | (see RTL)             | (see RTL)    | Compile-time random default constant for scrambling key (used in `prim_prince` block).
-`RndCnstRomNonce`           | (see RTL)             | (see RTL)    | Compile-time random default constant for scrambling nonce (used in `prim_prince` block and the two S&P blocks).
+`RndCnstRomNonce`           | (see RTL)             | (see RTL)    | Compile-time random default constant for scrambling nonce (used in `prim_prince` block and the S&P block).
 
 ### Signals
 

hw/ip/rom_ctrl/rtl/rom_ctrl_scrambled_rom.sv

@@ -148,22 +148,8 @@ module rom_ctrl_scrambled_rom
 
   assign scr_rdata_o = rdata_scr;
 
-  // Data scrambling ===========================================================
-
-  logic [Width-1:0] rdata_xor;
-
-  prim_subst_perm #(
-    .DataWidth (Width),
-    .NumRounds (2),
-    .Decrypt   (1)
-  ) u_sp_data (
-    .data_i (rdata_scr),
-    .key_i  ('0),
-    .data_o (rdata_xor)
-  );
-
   // XOR rdata with keystream ==================================================
 
-  assign clr_rdata_o = rdata_xor ^ keystream[Width-1:0];
+  assign clr_rdata_o = rdata_scr ^ keystream[Width-1:0];
 
 endmodule

hw/ip/sram_ctrl/doc/theory_of_operation.md

@@ -26,11 +26,8 @@ The individual mechanisms are explained in more detail in the subsections below.
 ## Scrambling Primitive
 
 As explained in [`prim_ram_1p_scr`](../../prim/doc/prim_ram_1p_scr.md) the scrambling mechanism employs a reduced-round PRINCE block cipher in CTR mode to scramble the data.
-Since plain CTR mode does not diffuse the data bits due to the bitwise XOR, the scheme is augmented by passing each word through a shallow substitution-permutation (S&P) network implemented with the `prim_subst_perm` primitive.
-The S&P network employed is similar to the one employed in PRESENT and is explained in more detail [here](../../prim/doc/prim_ram_1p_scr.md#custom-substitution-permutation-network).
 
-Another CTR mode augmentation that is aimed at breaking the linear address space is SRAM address scrambling.
-The same S&P network construction that is used for intra-word diffusion is leveraged to non-linearly remap the SRAM address as shown in the block diagram above.
+In order to break the linear address space, the CTR mode is augmented with an S&P network to non-linearly remap the SRAM address as shown in the block diagram above. The S&P network employed is similar to the one employed in PRESENT and is explained in more detail [here](../../prim/doc/prim_ram_1p_scr.md#custom-substitution-permutation-network).
 
 ### Integrity Error Handling
 

hw/ip/sram_ctrl/rtl/sram_ctrl.sv

@@ -492,8 +492,7 @@ module sram_ctrl
     .Width(DataWidth),
     .Depth(Depth),
     .EnableParity(0),
-    .DataBitsPerMask(DataWidth),
-    .DiffWidth(DataWidth)
+    .DataBitsPerMask(DataWidth)
   ) u_prim_ram_1p_scr (
     .clk_i,
     .rst_ni,