[flash_ctrl,dv] Make some structures unpacked

For example, consider flash_mp_region_cfg_t. Structs of this type are
randomised in several places (see flash_ctrl_error_mp_vseq, for
example). This isn't going to do what we need if the struct is packed.
For example (without further constraints), each of those seven mubi4_t
elements is going to be have an invalid encoding with probability
14/16.

This might be worked around. Indeed, I see that
flash_ctrl_error_mp_vseq has explicit constraints for the regions,
that will force the fields to take named enum values. But it's
probably more sensible to default to more guessable behaviour.

As well as flash_ctrl_error_mp_vseq, this commit makes the same change
to:

  - flash_bank_mp_info_page_cfg_t (another 7 mubi values, with the
    same analysis).

  - flash_op_t (contains several enums and gets confusingly randomised
    in several places, which is why I noticed this in the first place)

  - rd_cache_t (contains an enum variable)

It turns out that removing "packed" isn't quite enough. The biggest
reason is that there were quite a few "solve before" lines that aren't
actually allowed if you have something more interesting than a bit
vector.

Changes needed:

  - Make the fields of flash_op_t themselves be rand. This is needed
    because calling my_class.randomise(some_class_variable) doesn't
    work if some_class_variable is an unpacked struct whose fields
    aren't explicitly marked rand.

  - This is also needed in flash_mp_region_cfg_t and
    flash_bank_mp_info_page_cfg_t. For some reason, *that* isn't
    visible at compile time, but you end up with a runtime error where
    something is randomised with X values (not allowed by the spec!)
    and the root cause is that we're not actually randomising e.g. a
    region config struct, so it gets its initial value (of X).

  - Rephrase lots of rules of the form "solve xyz before
    flash_op" to things like "solve xyz before flash_op.addr". This is
    because the SystemVerilog spec doesn't allow anything except an
    integer in as a thing to be solved before/after. I think the DV
    engineer just has to pretend to be a compiler. Sigh...

  - Remove some ordering constraints of the form "solve en_mp_regions
    before mp_regions". This is definitely not allowed (because
    mp_regions is an array of unpacked structs, so not a bit vector).
    I'm not completely convinced we care but, if we do, my proposal
    would be to randomise the items of the vector explicitly as we get
    to them ("late randomisation"). The code ends up simpler, I think.

  - Fix flat-out bogus syntax in flash_ctrl_invalid_op_vseq.sv.
    Antonio and I have fixed the same error in quite a few places in
    the past. In this case, it came from commit 9b01461 in 2022.

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

Author: rswarbrick

hw/ip_templates/flash_ctrl/dv/env/flash_ctrl_env_pkg.sv

@@ -269,26 +269,26 @@ package flash_ctrl_env_pkg;
     NumReadTask  = 2
   } read_task_e;
 
-  typedef struct packed {
-    mubi4_t en;           // enable this region
-    mubi4_t read_en;      // enable reads
-    mubi4_t program_en;   // enable write
-    mubi4_t erase_en;     // enable erase
-    mubi4_t scramble_en;  // enable scramble
-    mubi4_t ecc_en;       // enable ecc
-    mubi4_t he_en;        // enable high endurance
-    uint    num_pages;    // 0:NumPages % start_page
-    uint    start_page;   // 0:NumPages-1
+  typedef struct {
+    rand mubi4_t en;           // enable this region
+    rand mubi4_t read_en;      // enable reads
+    rand mubi4_t program_en;   // enable write
+    rand mubi4_t erase_en;     // enable erase
+    rand mubi4_t scramble_en;  // enable scramble
+    rand mubi4_t ecc_en;       // enable ecc
+    rand mubi4_t he_en;        // enable high endurance
+    rand uint    num_pages;    // 0:NumPages % start_page
+    rand uint    start_page;   // 0:NumPages-1
   } flash_mp_region_cfg_t;
 
-  typedef struct packed {
-    mubi4_t en;           // enable this page
-    mubi4_t read_en;      // enable reads
-    mubi4_t program_en;   // enable write
-    mubi4_t erase_en;     // enable erase
-    mubi4_t scramble_en;  // enable scramble
-    mubi4_t ecc_en;       // enable ecc
-    mubi4_t he_en;        // enable high endurance
+  typedef struct {
+    rand mubi4_t en;           // enable this page
+    rand mubi4_t read_en;      // enable reads
+    rand mubi4_t program_en;   // enable write
+    rand mubi4_t erase_en;     // enable erase
+    rand mubi4_t scramble_en;  // enable scramble
+    rand mubi4_t ecc_en;       // enable ecc
+    rand mubi4_t he_en;        // enable high endurance
   } flash_bank_mp_info_page_cfg_t;
 
   // 2-states flash data type
@@ -307,20 +307,20 @@ package flash_ctrl_env_pkg;
   // Otf address in a bank.
   typedef bit [flash_ctrl_top_specific_pkg::BusAddrByteW-FlashBankWidth-1 : 0] otf_addr_t;
 
-  typedef struct packed {
-    flash_dv_part_e  partition;   // data or one of the info partitions
-    flash_erase_e    erase_type;  // erase page or the whole bank
-    flash_op_e       op;          // read / program or erase
-    flash_prog_sel_e prog_sel;    // program select: normal or repair
-    uint             num_words;   // number of words to read or program (TL_DW)
-    addr_t           addr;        // starting addr for the op
+  typedef struct {
+    rand flash_dv_part_e  partition;   // data or one of the info partitions
+    rand flash_erase_e    erase_type;  // erase page or the whole bank
+    rand flash_op_e       op;          // read / program or erase
+    rand flash_prog_sel_e prog_sel;    // program select: normal or repair
+    rand uint             num_words;   // number of words to read or program (TL_DW)
+    rand addr_t           addr;        // starting addr for the op
     // address for the ctrl interface per bank, 18:0
-    bit [flash_ctrl_top_specific_pkg::BusAddrByteW-2:0] otf_addr;
+    rand bit [flash_ctrl_top_specific_pkg::BusAddrByteW-2:0] otf_addr;
   } flash_op_t;
 
   // Address combined with region
   // Need for error injection.
-  typedef struct packed {
+  typedef struct {
     bit          bank;
     addr_t addr;
     flash_dv_part_e part;

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_basic_rw_vseq.sv

@@ -32,7 +32,7 @@ class flash_ctrl_basic_rw_vseq extends flash_ctrl_base_vseq;
 
   // Constraint for controller address to be in relevant range for the selected partition.
   constraint addr_c {
-    solve bank before flash_op;
+    solve bank before flash_op.addr;
     flash_op.addr inside {[BytesPerBank * bank : BytesPerBank * (bank + 1)]};
     flash_op.addr[2:0] == '0;
   }
@@ -52,7 +52,7 @@ class flash_ctrl_basic_rw_vseq extends flash_ctrl_base_vseq;
 
   // Flash ctrl operation data queue - used for programming or reading the flash.
   constraint flash_op_data_c {
-    solve flash_op before flash_op_data;
+    solve flash_op.num_words before flash_op_data;
 
     flash_op_data.size() == flash_op.num_words;
   }

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_erase_suspend_vseq.sv

@@ -71,8 +71,6 @@ class flash_ctrl_erase_suspend_vseq extends flash_ctrl_base_vseq;
   rand flash_mp_region_cfg_t mp_regions[flash_ctrl_top_specific_pkg::MpRegions];
 
   constraint mp_regions_c {
-    solve en_mp_regions before mp_regions;
-
     foreach (mp_regions[i]) {
       mp_regions[i].en == mubi4_bool_to_mubi(en_mp_regions[i]);
 

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_error_mp_vseq.sv

@@ -45,7 +45,7 @@ class flash_ctrl_error_mp_vseq extends flash_ctrl_base_vseq;
   // Constraint for controller address to be in the relevant range for
   // the selected partition.
   constraint addr_c {
-    solve bank before flash_op;
+    solve bank before flash_op.partition, flash_op.addr;
     flash_op.addr inside {[BytesPerBank * bank : BytesPerBank * (bank + 1)]};
     if (flash_op.partition != FlashPartData) {
       flash_op.addr inside
@@ -93,7 +93,7 @@ class flash_ctrl_error_mp_vseq extends flash_ctrl_base_vseq;
 
   // Flash ctrl operation data queue - used for programming or reading the flash.
   constraint flash_op_data_c {
-    solve flash_op before flash_op_data;
+    solve flash_op.num_words before flash_op_data;
     if (flash_op.op inside {flash_ctrl_top_specific_pkg::FlashOpRead,
                             flash_ctrl_top_specific_pkg::FlashOpProgram}) {
       flash_op_data.size() == flash_op.num_words;

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_error_prog_type_vseq.sv

@@ -41,7 +41,7 @@ class flash_ctrl_error_prog_type_vseq extends flash_ctrl_base_vseq;
 
   // Constraint for controller address to be in relevant range the for the selected partition.
   constraint addr_c {
-    solve bank before flash_op;
+    solve bank before flash_op.addr;
     flash_op.addr inside {[BytesPerBank * bank : BytesPerBank * (bank + 1)]};
   }
 

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_fetch_code_vseq.sv

@@ -52,7 +52,7 @@ class flash_ctrl_fetch_code_vseq extends flash_ctrl_base_vseq;
 
   // Constraint for controller address to be in relevant range for the selected partition.
   constraint addr_c {
-    solve bank before flash_op;
+    solve bank before flash_op.partition;
     flash_op.addr inside {[BytesPerBank * bank : BytesPerBank * (bank + 1)]};
     if (flash_op.partition != FlashPartData) {
       flash_op.addr inside
@@ -97,7 +97,7 @@ class flash_ctrl_fetch_code_vseq extends flash_ctrl_base_vseq;
 
   // Flash ctrl operation data queue - used for programming or reading the flash.
   constraint flash_op_data_c {
-    solve flash_op before flash_op_data;
+    solve flash_op.op, flash_op.num_words before flash_op_data;
     if (flash_op.op inside {
         flash_ctrl_top_specific_pkg::FlashOpRead, flash_ctrl_top_specific_pkg::FlashOpProgram}) {
       flash_op_data.size() == flash_op.num_words;
@@ -115,8 +115,6 @@ class flash_ctrl_fetch_code_vseq extends flash_ctrl_base_vseq;
   rand flash_mp_region_cfg_t mp_regions[flash_ctrl_top_specific_pkg::MpRegions];
 
   constraint mp_regions_c {
-    solve en_mp_regions before mp_regions;
-
     foreach (mp_regions[i]) {
       mp_regions[i].en == mubi4_bool_to_mubi(en_mp_regions[i]);
       mp_regions[i].read_en == MuBi4True;

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_host_dir_rd_vseq.sv

@@ -29,7 +29,7 @@ class flash_ctrl_host_dir_rd_vseq extends flash_ctrl_fetch_code_vseq;
 
   // Constraint address to be in relevant range for the selected partition.
   constraint addr_c {
-    solve bank before flash_op;
+    solve bank before flash_op.addr;
     flash_op.addr inside {[BytesPerBank * bank : BytesPerBank * (bank + 1) - BytesPerBank / 2]};
   }
 
@@ -45,8 +45,6 @@ class flash_ctrl_host_dir_rd_vseq extends flash_ctrl_fetch_code_vseq;
   data_t flash_rd_one_data;
 
   constraint mp_regions_c {
-    solve en_mp_regions before mp_regions;
-
     foreach (mp_regions[i]) {
       mp_regions[i].en == mubi4_bool_to_mubi(en_mp_regions[i]);
 

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_invalid_op_vseq.sv

@@ -29,7 +29,7 @@ class flash_ctrl_invalid_op_vseq extends flash_ctrl_base_vseq;
   bit expect_alert;
 
   constraint flash_op_data_c {
-    solve flash_op before flash_op_data;
+    solve flash_op.num_words before flash_op_data;
     flash_op_data.size() == flash_op.num_words;
   }
 
@@ -74,7 +74,7 @@ class flash_ctrl_invalid_op_vseq extends flash_ctrl_base_vseq;
   constraint mp_info_pages_c {
     foreach (mp_info_pages[i, j]) {
       mp_info_pages[i][j].size() == flash_ctrl_top_specific_pkg::InfoTypeSize[j];
-      foreach (mp_info_pages[i][j][k]) {
+      foreach (mp_info_pages[i, j, k]) {
         mp_info_pages[i][j][k].en == MuBi4True;
         mp_info_pages[i][j][k].read_en == MuBi4True;
         mp_info_pages[i][j][k].program_en == MuBi4True;

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_legacy_base_vseq.sv

@@ -11,7 +11,7 @@ class flash_ctrl_legacy_base_vseq extends flash_ctrl_otf_base_vseq;
 
   constraint rand_op_c {
     solve fractions before rand_op.addr;
-    solve flash_program_data before rand_op;
+    solve flash_program_data before rand_op.partition, rand_op.addr;
     solve rand_op.partition before rand_op.prog_sel, rand_op.addr;
     solve rand_op.addr before rand_op.otf_addr;
     solve rand_op.addr before rand_op.num_words;

hw/ip_templates/flash_ctrl/dv/env/seq_lib/flash_ctrl_mid_op_rst_vseq.sv

@@ -50,7 +50,7 @@ class flash_ctrl_mid_op_rst_vseq extends flash_ctrl_base_vseq;
   }
 
   constraint flash_op_data_c {
-    solve flash_op before flash_op_data;
+    solve flash_op.num_words before flash_op_data;
     flash_op_data.size() == flash_op.num_words;
   }