From 920db59b2e454f590ad3ebf74448ab4ca5010db3 Mon Sep 17 00:00:00 2001 From: Maciej Gibowicz Date: Tue, 24 Oct 2023 12:44:17 +0200 Subject: [PATCH] [BH-1791] Add CPU frequency lock during log dump This will improve security and peripheral stabilization when downloading logs to a file. --- harmony_changelog.md | 1 + module-sys/SystemManager/CMakeLists.txt | 1 + module-sys/SystemManager/LogSentinel.cpp | 69 +++++++++++++++++++ module-sys/SystemManager/PowerManager.cpp | 17 ++++- .../include/SystemManager/LogSentinel.hpp | 29 ++++++++ .../include/SystemManager/PowerManager.hpp | 3 + module-utils/log/CMakeLists.txt | 1 + module-utils/log/Logger.cpp | 10 +++ module-utils/log/Logger.hpp | 5 ++ pure_changelog.md | 1 + 10 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 module-sys/SystemManager/LogSentinel.cpp create mode 100644 module-sys/SystemManager/include/SystemManager/LogSentinel.hpp diff --git a/harmony_changelog.md b/harmony_changelog.md index 22e2a92b74c258f9e227b84c13e148f09047851c..068bdeaa498183bbc6012584a27bf18bd0e50f7d 100644 --- a/harmony_changelog.md +++ b/harmony_changelog.md @@ -11,6 +11,7 @@ * Fixed frequency lock during user activity * Fixed possibility of OS crash during update package size check * Fixed incorrect calculation of requested CPU frequency +* Fixed CPU frequency setting when dumping logs to a file ### Added * Files not fully transferred via Center will be now removed when USB cable is unplugged diff --git a/module-sys/SystemManager/CMakeLists.txt b/module-sys/SystemManager/CMakeLists.txt index 67be41169300e68109cf5bd9e9ef600cd49d6154..a239d4fc64a0c425846dcc042fef35f281e4e8b6 100644 --- a/module-sys/SystemManager/CMakeLists.txt +++ b/module-sys/SystemManager/CMakeLists.txt @@ -14,6 +14,7 @@ target_sources(sys-manager PRIVATE CpuGovernor.cpp CpuSentinel.cpp + LogSentinel.cpp GovernorSentinelOperations.cpp CpuStatistics.cpp TaskStatistics.cpp diff --git a/module-sys/SystemManager/LogSentinel.cpp b/module-sys/SystemManager/LogSentinel.cpp new file mode 100644 index 0000000000000000000000000000000000000000..d2fb1371ce6c422db58730ed1b0b4f93e6f00e99 --- /dev/null +++ b/module-sys/SystemManager/LogSentinel.cpp @@ -0,0 +1,69 @@ +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. +// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md + +#include +#include + +namespace sys +{ + namespace + { + constexpr std::uint32_t holdMutexTimeoutMs{100000}; + constexpr auto sentinelName{"LoggerSentinel"}; + constexpr auto holdFrequencyReason{"LogDump"}; + } // namespace + + LogSentinel::LogSentinel(bsp::CpuFrequencyMHz frequencyToLock) : requestedFrequency(frequencyToLock) + { + mutex = xSemaphoreCreateMutex(); + if (mutex == NULL) { + LOG_ERROR("Failed to create mutex for logger!"); + } + } + + void LogSentinel::HoldMinimumFrequency() + { + frequencyRequest = true; + if (mutex != NULL) { + if (xSemaphoreTake(mutex, pdMS_TO_TICKS(holdMutexTimeoutMs)) != pdTRUE) { + LOG_ERROR("Failed to get mutex for the logger!"); + } + } + frequencyRequest = false; + } + + void LogSentinel::ReleaseMinimumFrequency() + { + if (mutex != NULL) { + xSemaphoreGive(mutex); + } + } + + void LogSentinel::UpdateCurrentFrequency(bsp::CpuFrequencyMHz newFrequency) + { + const bool newPermission = newFrequency >= requestedFrequency; + if (mutex != NULL && dumpPermission != newPermission) { + if (newPermission) { + xSemaphoreGive(mutex); + } + else if (xSemaphoreTake(mutex, 0) != pdTRUE) { + LOG_ERROR("Failed to get mutex when reducing the CPU frequency!"); + return; + } + } + dumpPermission = newPermission; + } + + [[nodiscard]] auto LogSentinel::GetRequestedFrequency() const noexcept -> sentinel::View + { + sentinel::View view{.ownerTCBNumber = 0, + .name = sentinelName, + .minFrequency = (frequencyRequest || + ((mutex != NULL) && dumpPermission && (uxSemaphoreGetCount(mutex) == 0))) + ? requestedFrequency + : bsp::CpuFrequencyMHz::Level_0, + .reason = holdFrequencyReason}; + return view; + } + +} // namespace sys diff --git a/module-sys/SystemManager/PowerManager.cpp b/module-sys/SystemManager/PowerManager.cpp index 7acd341eda481f497f848045f387d1d10153eab2..ce8d09c57a4c4f7a2dfbee7c52434a4b3fcd30cf 100644 --- a/module-sys/SystemManager/PowerManager.cpp +++ b/module-sys/SystemManager/PowerManager.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include namespace sys @@ -19,6 +20,8 @@ namespace sys constexpr auto lowestLevelName{"lowestCpuFrequency"}; constexpr auto middleLevelName{"middleCpuFrequency"}; constexpr auto highestLevelName{"highestCpuFrequency"}; + + constexpr bsp::CpuFrequencyMHz logDumpFrequencyToHold{bsp::CpuFrequencyMHz::Level_4}; } // namespace CpuFrequencyMonitor::CpuFrequencyMonitor(const std::string &name) : levelName(name) @@ -60,6 +63,9 @@ namespace sys driverSEMC = drivers::DriverSEMC::Create(drivers::name::ExternalRAM); lowPowerControl = bsp::LowPowerMode::Create().value_or(nullptr); cpuGovernor = std::make_unique(); + logSentinel = std::make_unique(logDumpFrequencyToHold); + Log::Logger::get().preDumpToFile = [this]() { logSentinel->HoldMinimumFrequency(); }; + Log::Logger::get().postDumpToFile = [this]() { logSentinel->ReleaseMinimumFrequency(); }; cpuAlgorithms = std::make_unique(); cpuAlgorithms->emplace(sys::cpu::AlgoID::ImmediateUpscale, std::make_unique()); @@ -107,7 +113,7 @@ namespace sys uint32_t cpuLoad = cpuStatistics.GetPercentageCpuLoad(); cpu::UpdateResult retval; const cpu::AlgorithmData data{ - cpuLoad, lowPowerControl->GetCurrentFrequencyLevel(), cpuGovernor->GetMinimumFrequencyRequested()}; + cpuLoad, lowPowerControl->GetCurrentFrequencyLevel(), GetMinimumCpuFrequencyRequested()}; auto _ = gsl::finally([&retval, this, data] { retval.frequencySet = lowPowerControl->GetCurrentFrequencyLevel(); @@ -174,10 +180,19 @@ namespace sys UpdateCpuFrequencyMonitor(lowPowerControl->GetCurrentFrequencyLevel()); while (lowPowerControl->GetCurrentFrequencyLevel() != freq) { lowPowerControl->SetCpuFrequency(freq); + logSentinel->UpdateCurrentFrequency(freq); cpuGovernor->InformSentinelsAboutCpuFrequencyChange(freq); } } + [[nodiscard]] auto PowerManager::GetMinimumCpuFrequencyRequested() const noexcept -> sentinel::View + { + const auto governorSentinelsView = cpuGovernor->GetMinimumFrequencyRequested(); + const auto logSentinelView = logSentinel->GetRequestedFrequency(); + return (logSentinelView.minFrequency > governorSentinelsView.minFrequency) ? logSentinelView + : governorSentinelsView; + } + [[nodiscard]] auto PowerManager::getExternalRamDevice() const noexcept -> std::shared_ptr { return driverSEMC; diff --git a/module-sys/SystemManager/include/SystemManager/LogSentinel.hpp b/module-sys/SystemManager/include/SystemManager/LogSentinel.hpp new file mode 100644 index 0000000000000000000000000000000000000000..81860b6e0189df1c131f2868700c37797960e6f4 --- /dev/null +++ b/module-sys/SystemManager/include/SystemManager/LogSentinel.hpp @@ -0,0 +1,29 @@ +// Copyright (c) 2017-2023, Mudita Sp. z.o.o. All rights reserved. +// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md + +#pragma once + +#include +#include +#include "SentinelView.hpp" + +namespace sys +{ + class LogSentinel + { + public: + explicit LogSentinel(bsp::CpuFrequencyMHz frequencyToLock); + + void HoldMinimumFrequency(); + void ReleaseMinimumFrequency(); + void UpdateCurrentFrequency(bsp::CpuFrequencyMHz newFrequency); + [[nodiscard]] auto GetRequestedFrequency() const noexcept -> sentinel::View; + + private: + xSemaphoreHandle mutex; + bsp::CpuFrequencyMHz requestedFrequency{bsp::CpuFrequencyMHz::Level_0}; + bool dumpPermission{true}; + bool frequencyRequest{false}; + }; + +} // namespace sys diff --git a/module-sys/SystemManager/include/SystemManager/PowerManager.hpp b/module-sys/SystemManager/include/SystemManager/PowerManager.hpp index 454d3ed7276df6a232abb9ed67cc0c93c206be9c..6b45ee5c8db89ba8a60d61a656db056b975222c7 100644 --- a/module-sys/SystemManager/include/SystemManager/PowerManager.hpp +++ b/module-sys/SystemManager/include/SystemManager/PowerManager.hpp @@ -9,6 +9,7 @@ #include "drivers/semc/DriverSEMC.hpp" #include "SysCpuUpdateResult.hpp" #include "CpuGovernor.hpp" +#include "LogSentinel.hpp" #include "TaskStatistics.hpp" #include #include @@ -75,6 +76,7 @@ namespace sys void SetCpuFrequency(bsp::CpuFrequencyMHz freq); void UpdateCpuFrequencyMonitor(bsp::CpuFrequencyMHz currentFreq); + [[nodiscard]] auto GetMinimumCpuFrequencyRequested() const noexcept -> sentinel::View; TickType_t lastCpuFrequencyChangeTimestamp{0}; TickType_t lastLogStatisticsTimestamp{0}; @@ -84,6 +86,7 @@ namespace sys std::shared_ptr driverSEMC; std::unique_ptr lowPowerControl; std::unique_ptr cpuGovernor; + std::unique_ptr logSentinel; const bsp::PowerProfile powerProfile; std::unique_ptr cpuAlgorithms; diff --git a/module-utils/log/CMakeLists.txt b/module-utils/log/CMakeLists.txt index 7ad40848ab24abddcd370578d0375c6b7d8106b6..82f20f5a9e13e32ab2634fd0ff8731995687de11 100644 --- a/module-utils/log/CMakeLists.txt +++ b/module-utils/log/CMakeLists.txt @@ -20,6 +20,7 @@ target_link_libraries(log utility purefs-paths crashdump-metadata-store + Microsoft.GSL::GSL PUBLIC module-os log-api diff --git a/module-utils/log/Logger.cpp b/module-utils/log/Logger.cpp index d135b5184161bf9a108d5e1433698e9fbb87107b..8f2d45199ce77506d9c76b208397cdbfff908b77 100644 --- a/module-utils/log/Logger.cpp +++ b/module-utils/log/Logger.cpp @@ -7,6 +7,7 @@ #include #include #include +#include namespace { @@ -187,6 +188,15 @@ namespace Log /// @return: 1 - log flush successflul int Logger::dumpToFile(const std::filesystem::path &logDirectoryPath, LoggerState loggerState) { + if (preDumpToFile != nullptr) { + preDumpToFile(); + } + auto _ = gsl::finally([this] { + if (postDumpToFile != nullptr) { + postDumpToFile(); + } + }); + if (logFileName.empty()) { const auto &osMetadata = Store::CrashdumpMetadata::getInstance().getMetadataString(); logFileName = std::string(logFileNamePrefix) + logFileNameSeparator + osMetadata + logFileNameExtension; diff --git a/module-utils/log/Logger.hpp b/module-utils/log/Logger.hpp index 726682ff8d958495431c56189c41f37ca04b5bb8..20413ff86199af884873acdb7a96614aa826b77b 100644 --- a/module-utils/log/Logger.hpp +++ b/module-utils/log/Logger.hpp @@ -56,6 +56,11 @@ namespace Log int flushLogs(); [[nodiscard]] std::size_t getMaxLineLength(); + /// functions called immediately before and after dumping logs to a file + /// created to synchronize necessary peripherals with the CPU frequency + std::function preDumpToFile; + std::function postDumpToFile; + static constexpr auto CRIT_STR = "CRIT"; static constexpr auto IRQ_STR = "IRQ"; diff --git a/pure_changelog.md b/pure_changelog.md index 3991848c37b5bab6ed725a835b4f0a6cb8901d1e..c89ddeac1b136be22dfe3eab064ebb27c2253fb7 100644 --- a/pure_changelog.md +++ b/pure_changelog.md @@ -14,6 +14,7 @@ * Fixed possible crash when entering phone number * Fixed incorrect calculation of requested CPU frequency +* Fixed CPU frequency setting when dumping logs to a file ## [1.9.0 2023-10-19]