From 4280e0e371e228f74b146e7ee7609a6b945a00ac Mon Sep 17 00:00:00 2001 From: Joel Smith Date: Wed, 16 Oct 2024 09:50:24 -0700 Subject: [PATCH 1/3] Add common version handling code Add semver_t type into common to handle version numbers, based on semver 2.0.0 but more permissive since TT-KMD reports non-compliant version strings like "1.29" and "1.28-bh2". Add logic to PCIDevice class to report KMD version during device initialization. --- .github/workflows/run-tests.yml | 4 ++ common/semver.hpp | 73 +++++++++++++++++++++++++++++ device/pcie/pci_device.cpp | 20 +++++++- device/pcie/pci_device.hpp | 11 ++++- tests/CMakeLists.txt | 2 + tests/misc/CMakeLists.txt | 12 +++++ tests/misc/test_semver.cpp | 81 +++++++++++++++++++++++++++++++++ 7 files changed, 201 insertions(+), 2 deletions(-) create mode 100644 common/semver.hpp create mode 100644 tests/misc/CMakeLists.txt create mode 100644 tests/misc/test_semver.cpp diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index e9d1e6b2..e9935dcd 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -103,3 +103,7 @@ jobs: - name: Run API tests run: | ${{ env.TEST_OUTPUT_DIR }}/umd/api/api_tests + + - name: Run MISC tests + run: | + ${{ env.TEST_OUTPUT_DIR }}/umd/misc/umd_misc_tests diff --git a/common/semver.hpp b/common/semver.hpp new file mode 100644 index 00000000..76e181c0 --- /dev/null +++ b/common/semver.hpp @@ -0,0 +1,73 @@ +/* + * SPDX-FileCopyrightText: (c) 2024 Tenstorrent Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#pragma once + +#include +#include +#include + +#include "fmt/format.h" + +namespace tt::umd { + +/** + * Based on Semantic Versioning 2.0.0 (https://semver.org/) but more permissive. + * TT-KMD reports version strings that are technically not semver compliant. + */ + +class semver_t { +public: + const uint64_t major; + const uint64_t minor; + const uint64_t patch; + + semver_t(uint64_t major, uint64_t minor, uint64_t patch) : major(major), minor(minor), patch(patch) {} + + semver_t(const std::string& version_str) : semver_t(parse(version_str)) {} + + bool operator<(const semver_t& other) const { + return std::tie(major, minor, patch) < std::tie(other.major, other.minor, other.patch); + } + + bool operator>(const semver_t& other) const { return other < *this; } + + bool operator==(const semver_t& other) const { + return std::tie(major, minor, patch) == std::tie(other.major, other.minor, other.patch); + } + + bool operator!=(const semver_t& other) const { return !(*this == other); } + + bool operator<=(const semver_t& other) const { return !(other < *this); } + + bool operator>=(const semver_t& other) const { return !(*this < other); } + + std::string to_string() const { return fmt::format("{}.{}.{}", major, minor, patch); } + +private: + static semver_t parse(const std::string& version_str) { + std::istringstream iss(version_str); + std::string token; + uint64_t major = 0; + uint64_t minor = 0; + uint64_t patch = 0; + + if (std::getline(iss, token, '.')) { + major = std::stoull(token); + + if (std::getline(iss, token, '.')) { + minor = std::stoull(token); + + if (std::getline(iss, token, '.')) { + patch = std::stoull(token); + } + } + } + return semver_t(major, minor, patch); + } +}; + +} // namespace tt::umd diff --git a/device/pcie/pci_device.cpp b/device/pcie/pci_device.cpp index 23528e5a..badc10e1 100644 --- a/device/pcie/pci_device.cpp +++ b/device/pcie/pci_device.cpp @@ -258,7 +258,10 @@ PCIDevice::PCIDevice(int pci_device_number, int logical_device_id) : numa_node(read_sysfs(info, "numa_node")), revision(read_sysfs(info, "revision")), arch(detect_arch(info.device_id, revision)), - architecture_implementation(tt::umd::architecture_implementation::create(arch)) { + architecture_implementation(tt::umd::architecture_implementation::create(arch)), + kmd_version(read_kmd_version()) { + log_info(LogSiliconDriver, "Opened device {}; KMD version: {}", pci_device_num, kmd_version.to_string()); + struct { tenstorrent_query_mappings query_mappings; tenstorrent_mapping mapping_array[8]; @@ -809,3 +812,18 @@ void PCIDevice::print_file_contents(std::string filename, std::string hint) { } } } + +semver_t PCIDevice::read_kmd_version() { + static const std::string path = "/sys/module/tenstorrent/version"; + std::ifstream file(path); + + if (!file.is_open()) { + log_warning(LogSiliconDriver, "Failed to open file: {}", path); + return {0, 0, 0}; + } + + std::string version_str; + std::getline(file, version_str); + + return semver_t(version_str); +} diff --git a/device/pcie/pci_device.hpp b/device/pcie/pci_device.hpp index 455091bb..62ffb4c2 100644 --- a/device/pcie/pci_device.hpp +++ b/device/pcie/pci_device.hpp @@ -12,10 +12,12 @@ #include #include +#include "common/semver.hpp" #include "device/tlb.h" #include "device/tt_arch_types.h" #include "device/tt_cluster_descriptor_types.h" #include "device/tt_xy_pair.h" +#include "fmt/format.h" // TODO: this is used up in cluster.cpp but that logic ought to be // lowered into the PCIDevice class since it is specific to PCIe cards. @@ -31,7 +33,8 @@ constexpr unsigned int c_hang_read_value = 0xffffffffu; namespace tt::umd { class architecture_implementation; -} +struct semver_t; +} // namespace tt::umd struct dynamic_tlb { uint64_t bar_offset; // Offset that address is mapped to, within the PCI BAR. @@ -57,6 +60,9 @@ struct PciDeviceInfo { // onto this struct as methods? e.g. current_link_width etc. }; +// Do we want to put everything into this file into tt::umd namespace? +using tt::umd::semver_t; + class PCIDevice { const std::string device_path; // Path to character device: /dev/tenstorrent/N const int pci_device_num; // N in /dev/tenstorrent/N @@ -66,6 +72,7 @@ class PCIDevice { const int numa_node; // -1 if non-NUMA const int revision; // PCI revision value from sysfs const tt::ARCH arch; // e.g. Grayskull, Wormhole, Blackhole + const semver_t kmd_version; // KMD version std::unique_ptr architecture_implementation; public: @@ -230,5 +237,7 @@ class PCIDevice { // For debug purposes when various stages fails. void print_file_contents(std::string filename, std::string hint = ""); + semver_t read_kmd_version(); + std::vector hugepage_mapping_per_channel; }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f7524f36..bbc47fcd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -63,6 +63,7 @@ if(MASTER_PROJECT) endif() add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/api) add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/pcie) +add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/misc) add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/simulation) if($ENV{ARCH_NAME} STREQUAL "wormhole_b0") add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/wormhole) @@ -84,4 +85,5 @@ add_custom_target( simulation_tests test_pcie_device api_tests + umd_misc_tests ) diff --git a/tests/misc/CMakeLists.txt b/tests/misc/CMakeLists.txt new file mode 100644 index 00000000..fbde42df --- /dev/null +++ b/tests/misc/CMakeLists.txt @@ -0,0 +1,12 @@ +set(UMD_MISC_TESTS_SRCS test_semver.cpp) + +add_executable(umd_misc_tests ${UMD_MISC_TESTS_SRCS}) +target_link_libraries(umd_misc_tests PRIVATE test_common) +set_target_properties( + umd_misc_tests + PROPERTIES + RUNTIME_OUTPUT_DIRECTORY + ${CMAKE_BINARY_DIR}/test/umd/misc + OUTPUT_NAME + umd_misc_tests +) diff --git a/tests/misc/test_semver.cpp b/tests/misc/test_semver.cpp new file mode 100644 index 00000000..2a0217b7 --- /dev/null +++ b/tests/misc/test_semver.cpp @@ -0,0 +1,81 @@ +/* + * SPDX-FileCopyrightText: (c) 2024 Tenstorrent Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +#include "common/semver.hpp" + +using tt::umd::semver_t; + +TEST(Semver, Valid) { + const std::map valid_test_cases = { + {"1.29", semver_t(1, 29, 0)}, // technically invalid, but seen from TT-KMD + {"1.28-bh2", semver_t(1, 28, 0)}, // technically invalid, but seen from TT-KMD + {"0.0.4", semver_t(0, 0, 4)}, + {"1.2.3", semver_t(1, 2, 3)}, + {"10.20.30", semver_t(10, 20, 30)}, + {"1.1.2-prerelease+meta", semver_t(1, 1, 2)}, + {"1.1.2+meta", semver_t(1, 1, 2)}, + {"1.1.2+meta-valid", semver_t(1, 1, 2)}, + {"1.0.0-alpha", semver_t(1, 0, 0)}, + {"1.0.0-beta", semver_t(1, 0, 0)}, + {"1.0.0-alpha.beta", semver_t(1, 0, 0)}, + {"1.0.0-alpha.beta.1", semver_t(1, 0, 0)}, + {"1.0.0-alpha.1", semver_t(1, 0, 0)}, + {"1.0.0-alpha0.valid", semver_t(1, 0, 0)}, + {"1.0.0-alpha.0valid", semver_t(1, 0, 0)}, + {"1.0.0-alpha-a.b-c-somethinglong+build.1-aef.1-its-okay", semver_t(1, 0, 0)}, + {"1.0.0-rc.1+build.1", semver_t(1, 0, 0)}, + {"2.0.0-rc.1+build.123", semver_t(2, 0, 0)}, + {"1.2.3-beta", semver_t(1, 2, 3)}, + {"10.2.3-DEV-SNAPSHOT", semver_t(10, 2, 3)}, + {"1.2.3-SNAPSHOT-123", semver_t(1, 2, 3)}, + {"1.0.0", semver_t(1, 0, 0)}, + {"2.0.0", semver_t(2, 0, 0)}, + {"1.1.7", semver_t(1, 1, 7)}, + {"2.0.0+build.1848", semver_t(2, 0, 0)}, + {"2.0.1-alpha.1227", semver_t(2, 0, 1)}, + {"1.0.0-alpha+beta", semver_t(1, 0, 0)}, + {"1.2.3----RC-SNAPSHOT.12.9.1--.12+788", semver_t(1, 2, 3)}, + {"1.2.3----R-S.12.9.1--.12+meta", semver_t(1, 2, 3)}, + {"1.2.3----RC-SNAPSHOT.12.9.1--.12", semver_t(1, 2, 3)}, + {"1.0.0+0.build.1-rc.10000aaa-kk-0.1", semver_t(1, 0, 0)}, + {"1.0.0-0A.is.legal", semver_t(1, 0, 0)} + }; + + for (const auto &[version_str, expected] : valid_test_cases) { + semver_t actual(version_str); + EXPECT_EQ(actual.major, expected.major); + EXPECT_EQ(actual.minor, expected.minor); + EXPECT_EQ(actual.patch, expected.patch); + } +} + +TEST(Semver, Invalid) { + std::vector invalid_test_cases = { + "+invalid", + "-invalid", + "-invalid+invalid", + "-invalid.01", + "alpha", + "alpha.beta", + "alpha.beta.1", + "alpha.1", + "alpha+beta", + "alpha_beta", + "alpha.", + "alpha..", + "beta", + "-alpha.", + "+justmeta", + }; + + for (const auto &version_str : invalid_test_cases) { + EXPECT_THROW(semver_t{version_str}, std::exception) << "'" << version_str << "' should be invalid"; + } +} + From 1d480072a4e136ed6da9b6bdbb9064766e37218e Mon Sep 17 00:00:00 2001 From: Joel Smith Date: Mon, 18 Nov 2024 12:01:55 -0800 Subject: [PATCH 2/3] format --- tests/misc/test_semver.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/misc/test_semver.cpp b/tests/misc/test_semver.cpp index 2a0217b7..1d1a179a 100644 --- a/tests/misc/test_semver.cpp +++ b/tests/misc/test_semver.cpp @@ -5,6 +5,7 @@ */ #include + #include #include "common/semver.hpp" @@ -13,8 +14,8 @@ using tt::umd::semver_t; TEST(Semver, Valid) { const std::map valid_test_cases = { - {"1.29", semver_t(1, 29, 0)}, // technically invalid, but seen from TT-KMD - {"1.28-bh2", semver_t(1, 28, 0)}, // technically invalid, but seen from TT-KMD + {"1.29", semver_t(1, 29, 0)}, // technically invalid, but seen from TT-KMD + {"1.28-bh2", semver_t(1, 28, 0)}, // technically invalid, but seen from TT-KMD {"0.0.4", semver_t(0, 0, 4)}, {"1.2.3", semver_t(1, 2, 3)}, {"10.20.30", semver_t(10, 20, 30)}, @@ -44,8 +45,7 @@ TEST(Semver, Valid) { {"1.2.3----R-S.12.9.1--.12+meta", semver_t(1, 2, 3)}, {"1.2.3----RC-SNAPSHOT.12.9.1--.12", semver_t(1, 2, 3)}, {"1.0.0+0.build.1-rc.10000aaa-kk-0.1", semver_t(1, 0, 0)}, - {"1.0.0-0A.is.legal", semver_t(1, 0, 0)} - }; + {"1.0.0-0A.is.legal", semver_t(1, 0, 0)}}; for (const auto &[version_str, expected] : valid_test_cases) { semver_t actual(version_str); @@ -75,7 +75,6 @@ TEST(Semver, Invalid) { }; for (const auto &version_str : invalid_test_cases) { - EXPECT_THROW(semver_t{version_str}, std::exception) << "'" << version_str << "' should be invalid"; - } + EXPECT_THROW(semver_t{version_str}, std::exception) << "'" << version_str << "' should be invalid"; + } } - From 81044f24b6f3813af5dd6012c8c7ef1b442db269 Mon Sep 17 00:00:00 2001 From: Joel Smith Date: Sat, 23 Nov 2024 11:18:13 -0800 Subject: [PATCH 3/3] Improve log message --- device/pcie/pci_device.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/device/pcie/pci_device.cpp b/device/pcie/pci_device.cpp index badc10e1..d0427b3f 100644 --- a/device/pcie/pci_device.cpp +++ b/device/pcie/pci_device.cpp @@ -260,7 +260,7 @@ PCIDevice::PCIDevice(int pci_device_number, int logical_device_id) : arch(detect_arch(info.device_id, revision)), architecture_implementation(tt::umd::architecture_implementation::create(arch)), kmd_version(read_kmd_version()) { - log_info(LogSiliconDriver, "Opened device {}; KMD version: {}", pci_device_num, kmd_version.to_string()); + log_info(LogSiliconDriver, "Opened PCI device {}; KMD version: {}", pci_device_num, kmd_version.to_string()); struct { tenstorrent_query_mappings query_mappings;