[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.

    (I'm not convinced that this part of the change should be needed,
    but it can't hurt, even if it's working around a VCS problem).

  - 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,7 +269,7 @@ package flash_ctrl_env_pkg;
     NumReadTask  = 2
   } read_task_e;
 
-  typedef struct packed {
+  typedef struct {
     mubi4_t en;           // enable this region
     mubi4_t read_en;      // enable reads
     mubi4_t program_en;   // enable write
@@ -281,7 +281,7 @@ package flash_ctrl_env_pkg;
     uint    start_page;   // 0:NumPages-1
   } flash_mp_region_cfg_t;
 
-  typedef struct packed {
+  typedef struct {
     mubi4_t en;           // enable this page
     mubi4_t read_en;      // enable reads
     mubi4_t program_en;   // enable write
@@ -307,7 +307,7 @@ 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 {
+  typedef struct {
     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
@@ -320,7 +320,7 @@ package flash_ctrl_env_pkg;
 
   // 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;
   }