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

@@ -471,6 +471,7 @@ 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) {
+  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 +480,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 +532,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 +541,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 +551,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 +709,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 +1020,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,7 +1036,7 @@ 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) {
+  if (kern->num_accel > 0 && cra_ring_root_exist) {
     acl_kernel_cra_read(kern, 0, KERNEL_OFFSET_CSR, &csr);
     version = ACL_KERNEL_READ_BIT_RANGE(csr, KERNEL_CSR_LAST_VERSION_BIT,
                                         KERNEL_CSR_FIRST_VERSION_BIT);
@@ -1041,7 +1045,9 @@ int acl_kernel_if_post_pll_config_init(acl_kernel_if *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 +1154,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");
@@ -1222,9 +1229,8 @@ void acl_kernel_if_launch_kernel_on_custom_sof(
   } else {
     offset = (unsigned int)KERNEL_OFFSET_INVOCATION_IMAGE;
     image_p = (uintptr_t) & (image->work_dim);
-    image_size_static =
-        (size_t)((uintptr_t) & (image->arg_value) - (uintptr_t) &
-                 (image->work_dim));
+    image_size_static = (size_t)(
+        (uintptr_t) & (image->arg_value) - (uintptr_t) & (image->work_dim));
   }
 
   if ((kern->io.debug_verbosity) >= 2) {
@@ -1239,7 +1245,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 +1263,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);

src/acl_mem.cpp

@@ -438,9 +438,7 @@ CL_API_ENTRY cl_mem clCreateBufferWithPropertiesINTEL(
     case CL_MEM_ALLOC_BUFFER_LOCATION_INTEL: {
       tmp_mem_id = (cl_uint) * (properties + 1);
     } break;
-    default: {
-      BAIL_INFO(CL_INVALID_DEVICE, context, "Invalid properties");
-    }
+    default: { BAIL_INFO(CL_INVALID_DEVICE, context, "Invalid properties"); }
     }
     properties += 2;
   }
@@ -5569,8 +5567,8 @@ void acl_mem_migrate_buffer(void *user_data, acl_device_op_t *op) {
 
 #ifdef MEM_DEBUG_MSG
               printf("release block %zx (%u:%u) ",
-                     (size_t)(src_mem->reserved_allocations[src_device]
-                                                           [src_mem_id]),
+                     (size_t)(
+                         src_mem->reserved_allocations[src_device][src_mem_id]),
                      src_device, src_mem_id);
 #endif
               remove_mem_block_linked_list(
@@ -5956,9 +5954,8 @@ static void l_mem_transfer_buffer_explicitly(cl_context context,
           dst_unmap_cmd.info.mem_xfer.map_flags = CL_MAP_WRITE;
         }
         dst_unmap_cmd.info.mem_xfer.src_mem = context->unwrapped_host_mem;
-        dst_unmap_cmd.info.mem_xfer.src_offset[0] =
-            (size_t)((char *)dst_mem->host_mem.aligned_ptr -
-                     (char *)ACL_MEM_ALIGN);
+        dst_unmap_cmd.info.mem_xfer.src_offset[0] = (size_t)(
+            (char *)dst_mem->host_mem.aligned_ptr - (char *)ACL_MEM_ALIGN);
         dst_unmap_cmd.info.mem_xfer.src_offset[1] = 0;
         dst_unmap_cmd.info.mem_xfer.src_offset[2] = 0;
         dst_unmap_cmd.info.mem_xfer.dst_mem = dst_mem;
@@ -6322,9 +6319,8 @@ void acl_copy_device_buffers_to_host_before_programming(
       cmd.info.mem_xfer.src_offset[2] = 0;
       cmd.info.mem_xfer.dst_mem = context2->unwrapped_host_mem;
       if (mem->flags & CL_MEM_USE_HOST_PTR) {
-        cmd.info.mem_xfer.dst_offset[0] =
-            (size_t)((char *)mem->fields.buffer_objs.host_ptr -
-                     (char *)ACL_MEM_ALIGN);
+        cmd.info.mem_xfer.dst_offset[0] = (size_t)(
+            (char *)mem->fields.buffer_objs.host_ptr - (char *)ACL_MEM_ALIGN);
       } else {
         cmd.info.mem_xfer.dst_offset[0] =
             (size_t)((char *)mem->host_mem.aligned_ptr - (char *)ACL_MEM_ALIGN);
@@ -6468,9 +6464,8 @@ void acl_copy_device_buffers_from_host_after_programming(
       cmd.type = CL_COMMAND_WRITE_BUFFER;
       cmd.info.mem_xfer.src_mem = context2->unwrapped_host_mem;
       if (mem->flags & CL_MEM_USE_HOST_PTR) {
-        cmd.info.mem_xfer.src_offset[0] =
-            (size_t)((char *)mem->fields.buffer_objs.host_ptr -
-                     (char *)ACL_MEM_ALIGN);
+        cmd.info.mem_xfer.src_offset[0] = (size_t)(
+            (char *)mem->fields.buffer_objs.host_ptr - (char *)ACL_MEM_ALIGN);
       } else {
         cmd.info.mem_xfer.src_offset[0] =
             (size_t)((char *)mem->host_mem.aligned_ptr - (char *)ACL_MEM_ALIGN);

src/acl_printf.cpp

@@ -911,11 +911,13 @@ static size_t l_dump_printf_buffer(cl_event event, cl_kernel kernel,
       length = get_length_specifier(data_elem._conversion_string);
 
 #ifdef DEBUG
-      printf("length specifier=%s\n", length == LS_HL   ? "LS_HL"
-                                      : length == LS_HH ? "LS_HH"
-                                      : length == LS_H  ? "LS_H"
-                                      : length == LS_L  ? "LS_L"
-                                                        : "NONE");
+      printf("length specifier=%s\n",
+             length == LS_HL
+                 ? "LS_HL"
+                 : length == LS_HH
+                       ? "LS_HH"
+                       : length == LS_H ? "LS_H"
+                                        : length == LS_L ? "LS_L" : "NONE");
 #endif
 
       // the size of the data we are converting right now

src/acl_usm.cpp

@@ -112,9 +112,7 @@ CL_API_ENTRY void *CL_API_CALL clHostMemAllocINTEL(
     case CL_MEM_ALLOC_BUFFER_LOCATION_INTEL: {
       mem_id = (cl_uint) * (properties + 1);
     } break;
-    default: {
-      BAIL_INFO(CL_INVALID_PROPERTY, context, "Invalid properties");
-    }
+    default: { BAIL_INFO(CL_INVALID_PROPERTY, context, "Invalid properties"); }
     }
     properties += 2;
   }
@@ -267,9 +265,7 @@ clDeviceMemAllocINTEL(cl_context context, cl_device_id device,
     case CL_MEM_ALLOC_BUFFER_LOCATION_INTEL: {
       mem_id = (cl_uint) * (properties + 1);
     } break;
-    default: {
-      BAIL_INFO(CL_INVALID_DEVICE, context, "Invalid properties");
-    }
+    default: { BAIL_INFO(CL_INVALID_DEVICE, context, "Invalid properties"); }
     }
     properties += 2;
   }
@@ -421,9 +417,7 @@ clSharedMemAllocINTEL(cl_context context, cl_device_id device,
       }
       mem_id = (cl_uint) * (properties + 1);
     } break;
-    default: {
-      BAIL_INFO(CL_INVALID_PROPERTY, context, "Invalid properties");
-    }
+    default: { BAIL_INFO(CL_INVALID_PROPERTY, context, "Invalid properties"); }
     }
     properties += 2;
   }