Skip CSR version check when cra_ring_root doesn't exist

1. Parse the new field from the auto-discovery string
2. Changed csr_version to be optional.
3.Skip csr version check if cra_ring_root doesn't exit.
4.Update the forward-compatibility auto-discovery string test. Note:previous test had an issue: The number of fields per device global is fixed (here to 1), not variable. I had to update that as well.
5.Add new unit tests.

Author: zibaiwan

include/acl.h

@@ -526,6 +526,8 @@ typedef struct acl_device_def_autodiscovery_t {
   // Device global definition.
   std::unordered_map<std::string, acl_device_global_mem_def_t>
       device_global_mem_defs;
+  bool cra_ring_root_exist =
+      true; // Set the default value to true for backwards compatibility flows.
 } acl_device_def_autodiscovery_t;
 
 typedef struct acl_device_def_t {

include/acl_kernel_if.h

@@ -50,7 +50,9 @@ typedef struct {
 
   acl_bsp_io io;
 
-  unsigned int csr_version;
+  // csr_version is absent if there is no accelerators or cra_ring_root doesn't
+  // exist
+  std::optional<unsigned int> csr_version;
 
   // Depth of hardware kernel invocation queue
   unsigned int *accel_invoc_queue_depth;
@@ -69,7 +71,8 @@ int acl_kernel_if_init(acl_kernel_if *kern, acl_bsp_io bsp_io,
 int acl_kernel_if_update(const acl_device_def_autodiscovery_t &devdef,
                          acl_kernel_if *kern);
 int acl_kernel_if_is_valid(acl_kernel_if *kern);
-int acl_kernel_if_post_pll_config_init(acl_kernel_if *kern);
+int acl_kernel_if_post_pll_config_init(bool cra_ring_root_exist,
+                                       acl_kernel_if *kern);
 
 void acl_kernel_if_register_callbacks(
     acl_kernel_update_callback kernel_update,

src/acl_auto_configure.cpp

@@ -99,6 +99,25 @@ static bool read_uint_counters(const std::string &str,
   return true;
 }
 
+// Reads the next word in str and converts it into a boolean.
+// Returns true if a valid integer was read or false if an error occurred.
+// pos is updated to the position immediately following the parsed word
+// even if an error occurs.
+static bool read_bool_counters(const std::string &str,
+                               std::string::size_type &pos, bool &val,
+                               std::vector<int> &counters) noexcept {
+  std::string result;
+  pos = read_word(str, pos, result);
+  decrement_section_counters(counters);
+  try {
+    val = static_cast<bool>(std::stoi(result));
+  } catch (const std::exception &e) {
+    UNREFERENCED_PARAMETER(e);
+    return false;
+  }
+  return true;
+}
+
 // Reads the next word in str and converts it into an unsigned.
 // Returns true if a valid integer was read or false if an error occurred.
 // pos is updated to the position immediately following the parsed word
@@ -1052,6 +1071,12 @@ bool acl_load_device_def_from_str(const std::string &config_str,
         config_str, curr_pos, devdef.device_global_mem_defs, counters, err_str);
   }
 
+  // Check whether csr_ring_root exist in the IP.
+  if (result && counters.back() > 0) {
+    result = read_bool_counters(config_str, curr_pos,
+                                devdef.cra_ring_root_exist, counters);
+  }
+
   // forward compatibility: bypassing remaining fields at the end of device
   // description section
   while (result && counters.size() > 0 &&

src/acl_hal_mmd.cpp

@@ -1113,7 +1113,9 @@ static int l_try_device(unsigned int physical_device_id, const char *name,
 
   // Post-PLL config init function - at this point, it's safe to talk to the
   // kernel CSR registers.
-  if (acl_kernel_if_post_pll_config_init(&kern[physical_device_id]))
+  if (acl_kernel_if_post_pll_config_init(
+          sys->device[physical_device_id].autodiscovery_def.cra_ring_root_exist,
+          &kern[physical_device_id]))
     return 0;
 
   return 1;
@@ -2150,7 +2152,9 @@ int acl_hal_mmd_program_device(unsigned int physical_device_id,
   }
   // Post-PLL config init function - at this point, it's safe to talk to the
   // kernel CSR registers.
-  if (acl_kernel_if_post_pll_config_init(&kern[physical_device_id]))
+  if (acl_kernel_if_post_pll_config_init(
+          devdef->autodiscovery_def.cra_ring_root_exist,
+          &kern[physical_device_id]))
     return -1;
 
   return 0;

src/acl_kernel_if.cpp

@@ -470,7 +470,11 @@ static uintptr_t acl_kernel_cra_set_segment_rom(acl_kernel_if *kern,
 }
 
 static int acl_kernel_cra_read(acl_kernel_if *kern, unsigned int accel_id,
-                               unsigned int addr, unsigned int *val) {
+                               unsigned int addr, unsigned int *val,
+                               bool assert_csr_version = true) {
+  // Allow the very first reading csr_version call to run.
+  if (assert_csr_version)
+    assert(kern->csr_version);
   uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr);
   acl_assert_locked_or_sig();
   return acl_kernel_if_read_32b(
@@ -479,6 +483,7 @@ static int acl_kernel_cra_read(acl_kernel_if *kern, unsigned int accel_id,
 
 int acl_kernel_cra_read_64b(acl_kernel_if *kern, unsigned int accel_id,
                             unsigned int addr, uint64_t *val) {
+  assert(kern->csr_version);
   uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr);
   acl_assert_locked_or_sig();
   return acl_kernel_if_read_64b(
@@ -530,6 +535,7 @@ static int acl_kernel_rom_cra_read_block(acl_kernel_if *kern, unsigned int addr,
 
 static int acl_kernel_cra_write(acl_kernel_if *kern, unsigned int accel_id,
                                 unsigned int addr, unsigned int val) {
+  assert(kern->csr_version);
   uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr);
   acl_assert_locked_or_sig();
   return acl_kernel_if_write_32b(
@@ -538,6 +544,7 @@ static int acl_kernel_cra_write(acl_kernel_if *kern, unsigned int accel_id,
 
 static int acl_kernel_cra_write_64b(acl_kernel_if *kern, unsigned int accel_id,
                                     unsigned int addr, uint64_t val) {
+  assert(kern->csr_version);
   uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr);
   acl_assert_locked();
   return acl_kernel_if_write_64b(
@@ -547,6 +554,7 @@ static int acl_kernel_cra_write_64b(acl_kernel_if *kern, unsigned int accel_id,
 static int acl_kernel_cra_write_block(acl_kernel_if *kern,
                                       unsigned int accel_id, unsigned int addr,
                                       unsigned int *val, size_t size) {
+  assert(kern->csr_version);
   uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr);
   uintptr_t logical_addr =
       kern->accel_csr[accel_id].address + addr - OFFSET_KERNEL_CRA;
@@ -704,8 +712,6 @@ int acl_kernel_if_init(acl_kernel_if *kern, acl_bsp_io bsp_io,
   kern->accel_perf_mon = NULL;
   kern->accel_num_printfs = NULL;
 
-  kern->csr_version = 0;
-
   kern->autorun_profiling_kernel_id = -1;
 
   // The simulator doesn't have any kernel interface information until the aocx
@@ -1017,7 +1023,8 @@ int acl_kernel_if_update(const acl_device_def_autodiscovery_t &devdef,
 // Post-PLL config init function - at this point it's safe to talk to
 // the kernel CSR registers.
 // Returns 0 on success
-int acl_kernel_if_post_pll_config_init(acl_kernel_if *kern) {
+int acl_kernel_if_post_pll_config_init(bool cra_ring_root_exist,
+                                       acl_kernel_if *kern) {
   unsigned int csr, version;
   unsigned k;
   char *profile_start_cycle = getenv("ACL_PROFILE_START_CYCLE");
@@ -1032,16 +1039,18 @@ int acl_kernel_if_post_pll_config_init(acl_kernel_if *kern) {
     kern->io.printf("HAL Kern Error: Post PLL init: Invalid kernel handle");
     assert(0 && "Invalid kernel handle");
   }
-  if (kern->num_accel > 0) {
-    acl_kernel_cra_read(kern, 0, KERNEL_OFFSET_CSR, &csr);
+  if (kern->num_accel > 0 && cra_ring_root_exist) {
+    acl_kernel_cra_read(kern, 0, KERNEL_OFFSET_CSR, &csr, false);
     version = ACL_KERNEL_READ_BIT_RANGE(csr, KERNEL_CSR_LAST_VERSION_BIT,
                                         KERNEL_CSR_FIRST_VERSION_BIT);
     kern->csr_version = version;
     ACL_KERNEL_IF_DEBUG_MSG(kern,
                             "Read CSR version from kernel 0: Version = %u\n",
                             kern->csr_version);
   } else {
-    kern->csr_version = 0;
+    ACL_KERNEL_IF_DEBUG_MSG(kern,
+                            "No accelerator found or CRA ring root doesn't "
+                            "exist, not setting kern->csr_version\n");
   }
 
   // If environment variables set, configure the profile hardware
@@ -1148,7 +1157,8 @@ void acl_kernel_if_launch_kernel_on_custom_sof(
   // Version is defined in upper 16-bits of the status register and cached
   // in the kern->csr_version field.  The value is cached after PLL init to
   // avoid reading back on every kernel launch, which would add overhead.
-  if (!(kern->csr_version >= CSR_VERSION_ID_18_1 &&
+  if (kern->csr_version.has_value() &&
+      !(kern->csr_version >= CSR_VERSION_ID_18_1 &&
         kern->csr_version <= CSR_VERSION_ID)) {
     kern->io.printf("Hardware CSR version ID differs from version expected by "
                     "software.  Either:\n");
@@ -1239,7 +1249,8 @@ void acl_kernel_if_launch_kernel_on_custom_sof(
       }
     }
 
-    if (kern->csr_version != CSR_VERSION_ID_18_1) {
+    if (kern->csr_version.has_value() &&
+        (kern->csr_version != CSR_VERSION_ID_18_1)) {
       for (uintptr_t p = 0; p < image->arg_value_size; p += sizeof(int)) {
         unsigned int pword = *(unsigned int *)(image->arg_value + p);
         ACL_KERNEL_IF_DEBUG_MSG_VERBOSE(
@@ -1256,7 +1267,9 @@ void acl_kernel_if_launch_kernel_on_custom_sof(
     acl_kernel_cra_write_block(kern, accel_id, offset, (unsigned int *)image_p,
                                image_size_static);
   }
-  if (kern->csr_version != CSR_VERSION_ID_18_1 && image->arg_value_size > 0) {
+
+  if (kern->csr_version.has_value() &&
+      (kern->csr_version != CSR_VERSION_ID_18_1 && image->arg_value_size > 0)) {
     acl_kernel_cra_write_block(
         kern, accel_id, offset + (unsigned int)image_size_static,
         (unsigned int *)image->arg_value, image->arg_value_size);

test/acl_auto_configure_test.cpp

@@ -482,11 +482,11 @@ TEST(auto_configure, many_ok_forward_compatibility) {
   // sections and subsections to check forward compatibility
 
   std::string str(VERSIONIDTOSTR(
-      ACL_AUTO_CONFIGURE_VERSIONID) " 28 "
+      ACL_AUTO_CONFIGURE_VERSIONID) " 29 "
                                     "sample40byterandomhash000000000000000000 "
                                     "a10gx 0 1 15 DDR 2 1 6 0 2147483648 100 "
                                     "100 100 100 200 200 200 200 0 0 0 0 2 "
-                                    "1 name1 1 name2 47 "
+                                    "1 name1 name2 0 0 47 "
                                     "40 external_sort_stage_0 0 128 1 0 0 1 0 "
                                     "1 0 1 10 0 0 4 1 0 0 0 500 500 500 0 0 "
                                     "0 0 1 1 1 3 1 1 1 3 1 0 0 800 800 800 "
@@ -1410,3 +1410,63 @@ TEST(auto_configure, two_streaming_args_and_non_streaming_kernel) {
     CHECK(!args[i].streaming_arg_info_available);
   }
 }
+
+TEST(auto_configure, cra_ring_root_not_exist) {
+  const std::string config_str{
+      "23 50 2ccb683dee8e34a004c1a27e6d722090a8cc684d custom_ipa 0 1 9 0 2 1 2 "
+      "0 2199023255552 3 "
+      "- 0 6 5 ZTSZ4mainE4MyIP_arg_input_a 1 0 8 32768 "
+      "ZTSZ4mainE4MyIP_arg_input_b 1 0 8 32768 ZTSZ4mainE4MyIP_arg_input_c"
+      " 1 0 8 32768 ZTSZ4mainE4MyIP_arg_n 1 0 4 32768 "
+      "ZTSZ4mainE4MyIP_streaming_start 1 0 0 32768 "
+      "ZTSZ4mainE4MyIP_streaming_done"
+      " 0 1 0 32768 0 0 0 0 1 64 _ZTSZ4mainE4MyIP 0 128 1 0 0 1 0 1 0 4 8 2 1 "
+      "8 4 0 0 1 ZTSZ4mainE4MyIP_arg_input_a 8"
+      " 2 1 8 4 0 0 1 ZTSZ4mainE4MyIP_arg_input_b 8 2 1 8 4 0 0 1 "
+      "ZTSZ4mainE4MyIP_arg_input_c"
+      " 8 0 0 4 1 0 0 1 ZTSZ4mainE4MyIP_arg_n 0 0 0 0 1 1 1 3 1 1 1 3 1 1 1 "
+      "ZTSZ4mainE4MyIP_streaming_start"
+      " ZTSZ4mainE4MyIP_streaming_done"};
+
+  acl_device_def_autodiscovery_t devdef;
+  {
+    bool result;
+    std::string err_str;
+    ACL_LOCKED(result =
+                   acl_load_device_def_from_str(config_str, devdef, err_str));
+    std::cerr << err_str;
+    CHECK(result);
+  }
+
+  CHECK_EQUAL(0, devdef.cra_ring_root_exist);
+}
+
+TEST(auto_configure, cra_ring_root_exist) {
+  const std::string config_str{
+      "23 50 2ccb683dee8e34a004c1a27e6d722090a8cc684d custom_ipa 0 1 9 0 2 1 2 "
+      "0 2199023255552 3 "
+      "- 0 6 5 ZTSZ4mainE4MyIP_arg_input_a 1 0 8 32768 "
+      "ZTSZ4mainE4MyIP_arg_input_b 1 0 8 32768 ZTSZ4mainE4MyIP_arg_input_c"
+      " 1 0 8 32768 ZTSZ4mainE4MyIP_arg_n 1 0 4 32768 "
+      "ZTSZ4mainE4MyIP_streaming_start 1 0 0 32768 "
+      "ZTSZ4mainE4MyIP_streaming_done"
+      " 0 1 0 32768 0 0 0 1 1 64 _ZTSZ4mainE4MyIP 0 128 1 0 0 1 0 1 0 4 8 2 1 "
+      "8 4 0 0 1 ZTSZ4mainE4MyIP_arg_input_a 8"
+      " 2 1 8 4 0 0 1 ZTSZ4mainE4MyIP_arg_input_b 8 2 1 8 4 0 0 1 "
+      "ZTSZ4mainE4MyIP_arg_input_c"
+      " 8 0 0 4 1 0 0 1 ZTSZ4mainE4MyIP_arg_n 0 0 0 0 1 1 1 3 1 1 1 3 1 1 1 "
+      "ZTSZ4mainE4MyIP_streaming_start"
+      " ZTSZ4mainE4MyIP_streaming_done"};
+
+  acl_device_def_autodiscovery_t devdef;
+  {
+    bool result;
+    std::string err_str;
+    ACL_LOCKED(result =
+                   acl_load_device_def_from_str(config_str, devdef, err_str));
+    std::cerr << err_str;
+    CHECK(result);
+  }
+
+  CHECK_EQUAL(1, devdef.cra_ring_root_exist);
+}