From 81bb47a6d89f7fcb305149c350e1b3c6205f9e1f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 26 Jul 2023 11:05:36 +0200 Subject: [PATCH] this is a lot better --- bsp_q7s/boardtest/Q7STestTask.cpp | 27 +++-- bsp_q7s/core/CoreController.cpp | 177 ++++++------------------------ bsp_q7s/core/CoreController.h | 9 +- tmtc | 2 +- 4 files changed, 53 insertions(+), 162 deletions(-) diff --git a/bsp_q7s/boardtest/Q7STestTask.cpp b/bsp_q7s/boardtest/Q7STestTask.cpp index 50a34284..075ca1b3 100644 --- a/bsp_q7s/boardtest/Q7STestTask.cpp +++ b/bsp_q7s/boardtest/Q7STestTask.cpp @@ -218,15 +218,25 @@ void Q7STestTask::testProtHandler() { bool opPerformed = false; ReturnValue_t result = returnvalue::OK; // If any chips are unlocked, lock them here - result = coreController->setBootCopyProtection(xsc::Chip::ALL_CHIP, xsc::Copy::ALL_COPY, true, - opPerformed, true); + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_0, xsc::Copy::COPY_0, false); + if (result != returnvalue::OK) { + sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; + } + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_0, xsc::Copy::COPY_1, false); + if (result != returnvalue::OK) { + sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; + } + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_0, false); + if (result != returnvalue::OK) { + sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; + } + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, false); if (result != returnvalue::OK) { sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; } // unlock own copy - result = coreController->setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, false, - opPerformed, true); + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, false); if (result != returnvalue::OK) { sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; } @@ -239,8 +249,7 @@ void Q7STestTask::testProtHandler() { } // lock own copy - result = coreController->setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true, - opPerformed, true); + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true); if (result != returnvalue::OK) { sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; } @@ -253,8 +262,7 @@ void Q7STestTask::testProtHandler() { } // unlock specific copy - result = coreController->setBootCopyProtection(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, false, - opPerformed, true); + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, false); if (result != returnvalue::OK) { sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; } @@ -267,8 +275,7 @@ void Q7STestTask::testProtHandler() { } // lock specific copy - result = coreController->setBootCopyProtection(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, true, - opPerformed, true); + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, true); if (result != returnvalue::OK) { sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; } diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 6d8f60c5..e21a6417 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1232,16 +1232,15 @@ ReturnValue_t CoreController::gracefulShutdownTasks(xsc::Chip chip, xsc::Copy co // Attempt graceful shutdown by unmounting and switching off SD cards sdcMan->switchOffSdCard(sd::SdCard::SLOT_0); sdcMan->switchOffSdCard(sd::SdCard::SLOT_1); - // If any boot copies are unprotected. - // Actually this function only ensures that reboots to the own image are protected.. - ReturnValue_t result = setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true, - protOpPerformed, false); - if (result == returnvalue::OK and protOpPerformed) { + + // Ensure tha the target chip is writeprotected in any case. + bool wasProtected = handleBootCopyProt(chip, copy, true); + if (wasProtected) { // TODO: Would be nice to notify operator. But we can't use the filesystem anymore // and a reboot is imminent. Use scratch buffer? sif::info << "Running slot was writeprotected before reboot" << std::endl; } - return result; + return returnvalue::OK; } void CoreController::updateInternalSdInfo() { @@ -1274,144 +1273,50 @@ ReturnValue_t CoreController::generateChipStateFile() { return returnvalue::OK; } -ReturnValue_t CoreController::setBootCopyProtection(xsc::Chip targetChip, xsc::Copy targetCopy, - bool protect, bool &protOperationPerformed, - bool updateProtFile) { - bool allChips = false; - bool allCopies = false; - bool selfChip = false; - bool selfCopy = false; - protOperationPerformed = false; - - switch (targetChip) { - case (xsc::Chip::ALL_CHIP): { - allChips = true; - break; - } - case (xsc::Chip::NO_CHIP): { - return returnvalue::OK; - } - case (xsc::Chip::SELF_CHIP): { - selfChip = true; - targetChip = CURRENT_CHIP; - break; - } - default: { - break; - } - } - switch (targetCopy) { - case (xsc::Copy::ALL_COPY): { - allCopies = true; - break; - } - case (xsc::Copy::NO_COPY): { - return returnvalue::OK; - } - case (xsc::Copy::SELF_COPY): { - selfCopy = true; - targetCopy = CURRENT_COPY; - break; - } - default: { - break; - } +ReturnValue_t CoreController::setBootCopyProtectionAndUpdateFile(xsc::Chip targetChip, xsc::Copy targetCopy, + bool protect) { + if(targetChip == xsc::Chip::ALL_CHIP or targetCopy == xsc::Copy::ALL_COPY) { + return returnvalue::FAILED; } - for (uint8_t arrIdx = 0; arrIdx < protArray.size(); arrIdx++) { - int result = handleBootCopyProtAtIndex(targetChip, targetCopy, protect, protOperationPerformed, - selfChip, selfCopy, allChips, allCopies, arrIdx); - if (result != 0) { - break; - } - } - if (protOperationPerformed and updateProtFile) { + bool protOperationPerformed = handleBootCopyProt(targetChip, targetCopy, protect); + if (protOperationPerformed) { updateProtInfo(); } return returnvalue::OK; } -int CoreController::handleBootCopyProtAtIndex(xsc::Chip targetChip, xsc::Copy targetCopy, - bool protect, bool &protOperationPerformed, - bool selfChip, bool selfCopy, bool allChips, - bool allCopies, uint8_t arrIdx) { - bool currentProt = protArray[arrIdx]; +bool CoreController::handleBootCopyProt(xsc::Chip targetChip, xsc::Copy targetCopy, + bool protect) { std::ostringstream oss; - bool performOp = false; - if (protect == currentProt) { - return 0; - } - if (protOperationPerformed) { - if ((selfChip and selfCopy) or (not allCopies and not allChips)) { - // No need to continue, only one operation was requested - return 1; - } - } - xsc::Chip currentChip; - xsc::Copy currentCopy; oss << "writeprotect "; - if (arrIdx == 0 or arrIdx == 1) { - oss << "0 "; - currentChip = xsc::Chip::CHIP_0; - } else { - oss << "1 "; - currentChip = xsc::Chip::CHIP_1; + if(targetChip == xsc::Chip::SELF_CHIP) { + targetChip = CURRENT_CHIP; } - if (arrIdx == 0 or arrIdx == 2) { + if(targetCopy == xsc::Copy::SELF_COPY) { + targetCopy = CURRENT_COPY; + } + if (targetChip == xsc::Chip::CHIP_0) { oss << "0 "; - currentCopy = xsc::Copy::COPY_0; - } else { + } else if(targetChip == xsc::Chip::SELF_CHIP) { + oss << "1 "; + } + if(targetCopy == xsc::Copy::COPY_0) { + oss << "0 "; + } else if(targetCopy == xsc::Copy::COPY_1) { oss << "1 "; - currentCopy = xsc::Copy::COPY_1; } if (protect) { oss << "1"; } else { oss << "0"; } - - int result = 0; - if (allChips and allCopies) { - performOp = true; - } else if (allChips) { - if ((selfCopy and CURRENT_COPY == targetCopy) or (currentCopy == targetCopy)) { - performOp = true; - } - } else if (allCopies) { - if ((selfChip and CURRENT_COPY == targetCopy) or (currentChip == targetChip)) { - performOp = true; - } - } else if (selfChip and (currentChip == targetChip)) { - if (selfCopy) { - if (currentCopy == targetCopy) { - performOp = true; - } - } else { - performOp = true; - } - - } else if (selfCopy and (currentCopy == targetCopy)) { - if (selfChip) { - if (currentChip == targetChip) { - performOp = true; - } - } else { - performOp = true; - } - } else if ((targetChip == currentChip) and (targetCopy == currentCopy)) { - performOp = true; + sif::info << "Executing command: " << oss.str() << std::endl; + int result = std::system(oss.str().c_str()); + if(result == 0) { + return true; } - if (result != 0) { - utility::handleSystemError(result, "CoreController::checkAndSetBootCopyProtection"); - } - if (performOp) { - // TODO: Lock operation take a long time. Use command executor? That would require a - // new state machine.. - protOperationPerformed = true; - sif::info << "Executing command: " << oss.str() << std::endl; - result = std::system(oss.str().c_str()); - } - return 0; + return false; } ReturnValue_t CoreController::updateProtInfo(bool regenerateChipStateFile) { @@ -1456,7 +1361,6 @@ ReturnValue_t CoreController::handleProtInfoUpdateLine(std::string nextLine) { using namespace std; string word; uint8_t wordIdx = 0; - uint8_t arrayIdx = 0; istringstream iss(nextLine); xsc::Chip currentChip = xsc::Chip::CHIP_0; xsc::Copy currentCopy = xsc::Copy::COPY_0; @@ -1468,28 +1372,11 @@ ReturnValue_t CoreController::handleProtInfoUpdateLine(std::string nextLine) { currentCopy = static_cast(stoi(word)); } - if (wordIdx == 3) { - if (currentChip == xsc::Chip::CHIP_0) { - if (currentCopy == xsc::Copy::COPY_0) { - arrayIdx = 0; - } else if (currentCopy == xsc::Copy::COPY_1) { - arrayIdx = 1; - } - } - - else if (currentChip == xsc::Chip::CHIP_1) { - if (currentCopy == xsc::Copy::COPY_0) { - arrayIdx = 2; - } else if (currentCopy == xsc::Copy::COPY_1) { - arrayIdx = 3; - } - } - } if (wordIdx == 5) { if (word == "unlocked.") { - protArray[arrayIdx] = false; + protArray[currentChip][currentCopy] = false; } else { - protArray[arrayIdx] = true; + protArray[currentChip][currentCopy] = true; } } wordIdx++; diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index b6382d36..6f7b83e6 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -199,8 +199,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe * @param updateProtFile Specify whether the protection info file is updated * @return */ - ReturnValue_t setBootCopyProtection(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect, - bool& protOperationPerformed, bool updateProtFile = true); + ReturnValue_t setBootCopyProtectionAndUpdateFile(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect); bool sdInitFinished() const; @@ -309,7 +308,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe * Index 2: Chip 1 Copy 0 * Index 3: Chip 1 Copy 1 */ - std::array protArray; + bool protArray[2][2] {}; PeriodicOperationDivider opDivider5; PeriodicOperationDivider opDivider10; @@ -374,9 +373,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe ReturnValue_t gracefulShutdownTasks(xsc::Chip chip, xsc::Copy copy, bool& protOpPerformed); ReturnValue_t handleProtInfoUpdateLine(std::string nextLine); - int handleBootCopyProtAtIndex(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect, - bool& protOperationPerformed, bool selfChip, bool selfCopy, - bool allChips, bool allCopies, uint8_t arrIdx); + bool handleBootCopyProt(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect); void rebootWatchdogAlgorithm(RebootWatchdogFile& rf, bool& needsReboot, xsc::Chip& tgtChip, xsc::Copy& tgtCopy); void resetRebootWatchdogCounters(xsc::Chip tgtChip, xsc::Copy tgtCopy); diff --git a/tmtc b/tmtc index a82cbff5..95b69541 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit a82cbff5a83eb37c68234bdeecd3b7d308d65eb1 +Subproject commit 95b69541751c9ea7146e0a342122da83bdb50f29