diff --git a/include/acl.h b/include/acl.h index 6f5e24c3..ee98f9a5 100644 --- a/include/acl.h +++ b/include/acl.h @@ -526,6 +526,8 @@ typedef struct acl_device_def_autodiscovery_t { // Device global definition. std::unordered_map 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 { diff --git a/include/acl_kernel_if.h b/include/acl_kernel_if.h index 1b11679f..80a56683 100644 --- a/include/acl_kernel_if.h +++ b/include/acl_kernel_if.h @@ -50,7 +50,28 @@ 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 csr_version; + + // The real kern->cra_ring_root_exist value is initialized in the + // acl_kernel_if_update() function (reading the actual auto-discovery string + // at that time). + // However, the kern->cra_ring_root_exist is referenced in the + // acl_kernel_if_post_pll_config_init() to check whether it's OK to read from + // the CSR. And acl_kernel_if_post_pll_config_init() can be called prior to + // the acl_kernel_if_update(), using a dummy auto-discovery string. + // Therefore we must set a default value. (Otherwise, it's reading an + // uninitialized value) + + // The reason of setting it to false: + // Currently the cra read function will not be called in the early + // acl_kernel_if_post_pll_config_init() call, because the kern->num_accel is 0 + // with dummy auto-discovery string, there is an if statement guard that. With + // the default value to false, it will hit the assertions in the cra + // read/write functions in case those are accidentally invoked too early, + // e.g., in a future code refactoring. + bool cra_ring_root_exist = false; // Depth of hardware kernel invocation queue unsigned int *accel_invoc_queue_depth; diff --git a/src/acl_auto_configure.cpp b/src/acl_auto_configure.cpp index 034338be..323cba8e 100644 --- a/src/acl_auto_configure.cpp +++ b/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 &counters) noexcept { + std::string result; + pos = read_word(str, pos, result); + decrement_section_counters(counters); + try { + val = static_cast(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 && diff --git a/src/acl_kernel_if.cpp b/src/acl_kernel_if.cpp index f46a1e42..bbb5f6c0 100644 --- a/src/acl_kernel_if.cpp +++ b/src/acl_kernel_if.cpp @@ -477,6 +477,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->cra_ring_root_exist); uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr); acl_assert_locked_or_sig(); return acl_kernel_if_read_32b( @@ -485,6 +486,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->cra_ring_root_exist); uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr); acl_assert_locked_or_sig(); return acl_kernel_if_read_64b( @@ -536,6 +538,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->cra_ring_root_exist); uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr); acl_assert_locked_or_sig(); return acl_kernel_if_write_32b( @@ -544,6 +547,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->cra_ring_root_exist); uintptr_t segment_offset = acl_kernel_cra_set_segment(kern, accel_id, addr); acl_assert_locked(); return acl_kernel_if_write_64b( @@ -553,6 +557,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->cra_ring_root_exist); 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; @@ -712,8 +717,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 @@ -897,6 +900,11 @@ int acl_kernel_if_update(const acl_device_def_autodiscovery_t &devdef, acl_kernel_if_close(kern); + // Initialize whether the cra_ring_root exist in the design + kern->cra_ring_root_exist = devdef.cra_ring_root_exist; + ACL_KERNEL_IF_DEBUG_MSG(kern, "Number of cra_ring_root : %d\n", + kern->cra_ring_root_exist); + // Setup the PCIe HAL Structures kern->num_accel = static_cast(devdef.accel.size()); ACL_KERNEL_IF_DEBUG_MSG(kern, "Number of Accelerators : %d\n", @@ -1040,7 +1048,8 @@ 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 && kern->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); @@ -1054,7 +1063,9 @@ int acl_kernel_if_post_pll_config_init(acl_kernel_if *kern) { kern->cra_address_offset = 0; } } 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 @@ -1161,7 +1172,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"); @@ -1253,7 +1265,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( @@ -1270,7 +1283,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); diff --git a/test/acl_auto_configure_test.cpp b/test/acl_auto_configure_test.cpp index cf924dc9..fe1aa742 100644 --- a/test/acl_auto_configure_test.cpp +++ b/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); +}