From e0cfdb27c57fa4ea0f25315f37558e96d0241c02 Mon Sep 17 00:00:00 2001 From: Akshay Tondak Date: Thu, 3 Oct 2024 15:33:49 -0700 Subject: [PATCH] Fix for CR-1214058 : xrt-smi shows Alveo help when invoked with invalid args (#8483) * Fix for CR-1214058 Signed-off-by: Akshay Tondak * Added empty elements check to make the behavior uniform Signed-off-by: Akshay Tondak * Added operation cancelled string back since it didn't make xrt-smi to fail completely Signed-off-by: Akshay Tondak * Error handling code cleanup for xrt-smi validate Signed-off-by: Akshay Tondak --------- Signed-off-by: Akshay Tondak Co-authored-by: Akshay Tondak --- .../tools/common/SubCmdExamineInternal.cpp | 5 +- .../core/tools/xbutil2/SubCmdValidate.cpp | 120 ++++++++++-------- .../core/tools/xbutil2/SubCmdValidate.h | 4 + 3 files changed, 76 insertions(+), 53 deletions(-) diff --git a/src/runtime_src/core/tools/common/SubCmdExamineInternal.cpp b/src/runtime_src/core/tools/common/SubCmdExamineInternal.cpp index 29e6f5f52ba..dd6f1092403 100644 --- a/src/runtime_src/core/tools/common/SubCmdExamineInternal.cpp +++ b/src/runtime_src/core/tools/common/SubCmdExamineInternal.cpp @@ -77,7 +77,7 @@ SubCmdExamineInternal::SubCmdExamineInternal(bool _isHidden, bool _isDepricated, if (m_isUserDomain) m_hiddenOptions.add_options() - ("element,e", boost::program_options::value(&m_elementsFilter)->multitoken(), "Filters individual elements(s) from the report. Format: '///...'") + ("element,e", boost::program_options::value(&m_elementsFilter)->multitoken()->zero_tokens(), "Filters individual elements(s) from the report. Format: '///...'") ; } @@ -138,6 +138,9 @@ SubCmdExamineInternal::execute(const SubCmdOptions& _options) const if (vm.count("report") && m_reportNames.empty()) throw xrt_core::error("No report given to be produced"); + if (vm.count("element") && m_elementsFilter.empty()) + throw xrt_core::error("No element filter given to be produced"); + // DRC check // When json is specified, make sure an accompanying output file is also specified if (!m_format.empty() && m_output.empty()) diff --git a/src/runtime_src/core/tools/xbutil2/SubCmdValidate.cpp b/src/runtime_src/core/tools/xbutil2/SubCmdValidate.cpp index 162ff8ce7cb..67091946615 100644 --- a/src/runtime_src/core/tools/xbutil2/SubCmdValidate.cpp +++ b/src/runtime_src/core/tools/xbutil2/SubCmdValidate.cpp @@ -480,15 +480,15 @@ SubCmdValidate::SubCmdValidate(bool _isHidden, bool _isDepricated, bool _isPreli common_options.add_options() ("device,d", boost::program_options::value(&m_device), "The Bus:Device.Function (e.g., 0000:d8:00.0) device of interest") - ("format,f", boost::program_options::value(&m_format), (std::string("Report output format. Valid values are:\n") + formatOptionValues).c_str() ) - ("output,o", boost::program_options::value(&m_output), "Direct the output to the given file") - ("pmode", boost::program_options::value(&m_pmode), "Specify which power mode to run the benchmarks in. Note: Some tests might be unavailable for some modes") + ("format,f", boost::program_options::value(&m_format)->implicit_value(""), (std::string("Report output format. Valid values are:\n") + formatOptionValues).c_str() ) + ("output,o", boost::program_options::value(&m_output)->implicit_value(""), "Direct the output to the given file") + ("pmode", boost::program_options::value(&m_pmode)->implicit_value(""), "Specify which power mode to run the benchmarks in. Note: Some tests might be unavailable for some modes") ("help", boost::program_options::bool_switch(&m_help), "Help to use this sub-command") ; m_hiddenOptions.add_options() ("path,p", boost::program_options::value(&m_xclbin_location)->implicit_value(""), "Path to the directory containing validate xclbins") - ("param", boost::program_options::value(&m_param), (std::string("Extended parameter for a given test. Format: ::\n") + extendedKeysOptions()).c_str()) + ("param", boost::program_options::value(&m_param)->implicit_value(""), (std::string("Extended parameter for a given test. Format: ::\n") + extendedKeysOptions()).c_str()) ; m_commonOptions.add(common_options); @@ -520,6 +520,68 @@ SubCmdValidate::print_help_internal() const printHelp(common_options, m_hiddenOptions, deviceClass, false); } +void +SubCmdValidate::handle_errors_and_validate_tests(po::variables_map& vm, + std::vector& validatedTests, + std::vector& param) const +{ + const auto testNameDescription = getTestNameDescriptions(true /* Add "all" and "quick" options*/); + + if (vm.count("output") && m_output.empty()) + throw xrt_core::error("Output file not specified"); + + if (vm.count("path") && m_xclbin_location.empty()) + throw xrt_core::error("xclbin path not specified"); + + if (vm.count("param") && m_param.empty()) + throw xrt_core::error("Parameter not specified"); + + if (vm.count("pmode") && m_pmode.empty()) + throw xrt_core::error("Power mode not specified"); + + if (vm.count("format") && m_format.empty()) + throw xrt_core::error("Output format not specified"); + + if (!m_output.empty() && !XBU::getForce() && std::filesystem::exists(m_output)) + throw xrt_core::error((boost::format("Output file already exists: '%s'") % m_output).str()); + + if (m_tests_to_run.empty()) + throw xrt_core::error("No test given to validate against."); + + // Validate the user test requests + for (auto &userTestName : m_tests_to_run) { + const auto validateTestName = boost::algorithm::to_lower_copy(userTestName); + + if ((validateTestName == "all") && (m_tests_to_run.size() > 1)) + throw xrt_core::error("The 'all' value for the tests to run cannot be used with any other named tests."); + + if ((validateTestName == "quick") && (m_tests_to_run.size() > 1)) + throw xrt_core::error("The 'quick' value for the tests to run cannot be used with any other name tests."); + + // Verify the current user test request exists in the test suite + doesTestExist(validateTestName, testNameDescription); + validatedTests.push_back(validateTestName); + } + //check if param option is provided + if (!m_param.empty()) { + XBU::verbose("Sub command: --param"); + boost::split(param, m_param, boost::is_any_of(":")); // eg: dma:block-size:1024 + + //check parameter format + if (param.size() != 3) + throw xrt_core::error((boost::format("Invalid parameter format (expected 3 positional arguments): '%s'") % m_param).str()); + + //check test case name + doesTestExist(param[0], testNameDescription); + + //check parameter name + auto iter = std::find_if( extendedKeysCollection.begin(), extendedKeysCollection.end(), + [¶m](const ExtendedKeysStruct& collection){ return collection.param_name == param[1];} ); + if (iter == extendedKeysCollection.end()) + throw xrt_core::error((boost::format("Unsupported parameter name '%s' for validation test '%s'") % param[1] % param[2]).str()); + } +} + void SubCmdValidate::execute(const SubCmdOptions& _options) const { @@ -555,40 +617,14 @@ SubCmdValidate::execute(const SubCmdOptions& _options) const std::vector param; std::vector validatedTests; std::string validateXclbinPath = m_xclbin_location; - const auto testNameDescription = getTestNameDescriptions(true /* Add "all" and "quick" options*/); try { // Output Format schemaVersion = Report::getSchemaDescription(m_format).schemaVersion; if (schemaVersion == Report::SchemaVersion::unknown) throw xrt_core::error((boost::format("Unknown output format: '%s'") % m_format).str()); - // Output file - if (vm.count("output") && m_output.empty()) - throw xrt_core::error("Output file not specified"); - - if (vm.count("path") && m_xclbin_location.empty()) - throw xrt_core::error("xclbin path not specified"); - - if (!m_output.empty() && !XBU::getForce() && std::filesystem::exists(m_output)) - throw xrt_core::error((boost::format("Output file already exists: '%s'") % m_output).str()); - - if (m_tests_to_run.empty()) - throw xrt_core::error("No test given to validate against."); - - // Validate the user test requests - for (auto &userTestName : m_tests_to_run) { - const auto validateTestName = boost::algorithm::to_lower_copy(userTestName); - - if ((validateTestName == "all") && (m_tests_to_run.size() > 1)) - throw xrt_core::error("The 'all' value for the tests to run cannot be used with any other named tests."); - - if ((validateTestName == "quick") && (m_tests_to_run.size() > 1)) - throw xrt_core::error("The 'quick' value for the tests to run cannot be used with any other name tests."); - - // Verify the current user test request exists in the test suite - doesTestExist(validateTestName, testNameDescription); - validatedTests.push_back(validateTestName); - } + // All Error Handling for xrt-smi validate should go here + handle_errors_and_validate_tests(vm, validatedTests, param); // check if xclbin folder path is provided if (!validateXclbinPath.empty()) { @@ -600,26 +636,6 @@ SubCmdValidate::execute(const SubCmdOptions& _options) const if (validateXclbinPath.back() != '/') validateXclbinPath.append("/"); } - - //check if param option is provided - if (!m_param.empty()) { - XBU::verbose("Sub command: --param"); - boost::split(param, m_param, boost::is_any_of(":")); // eg: dma:block-size:1024 - - //check parameter format - if (param.size() != 3) - throw xrt_core::error((boost::format("Invalid parameter format (expected 3 positional arguments): '%s'") % m_param).str()); - - //check test case name - doesTestExist(param[0], testNameDescription); - - //check parameter name - auto iter = std::find_if( extendedKeysCollection.begin(), extendedKeysCollection.end(), - [¶m](const ExtendedKeysStruct& collection){ return collection.param_name == param[1];} ); - if (iter == extendedKeysCollection.end()) - throw xrt_core::error((boost::format("Unsupported parameter name '%s' for validation test '%s'") % param[1] % param[2]).str()); - } - } catch (const xrt_core::error& e) { // Catch only the exceptions that we have generated earlier std::cerr << boost::format("ERROR: %s\n") % e.what(); diff --git a/src/runtime_src/core/tools/xbutil2/SubCmdValidate.h b/src/runtime_src/core/tools/xbutil2/SubCmdValidate.h index c28169959d3..6efe9681a29 100644 --- a/src/runtime_src/core/tools/xbutil2/SubCmdValidate.h +++ b/src/runtime_src/core/tools/xbutil2/SubCmdValidate.h @@ -5,6 +5,7 @@ #ifndef __SubCmdValidate_h_ #define __SubCmdValidate_h_ +#include #include "tools/common/SubCmd.h" #include "tools/common/XBHelpMenus.h" @@ -26,6 +27,9 @@ class SubCmdValidate : public SubCmd { bool m_help; void print_help_internal() const; + void handle_errors_and_validate_tests(boost::program_options::variables_map&, + std::vector&, + std::vector&) const; XBUtilities::VectorPairStrings getTestNameDescriptions(const bool addAdditionOptions) const; };