diff --git a/CHANGELOG.md b/CHANGELOG.md index 1329d338..0a32d0f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ will consitute of a breaking change warranting a new major release: from SUS and MGM measurements. To accommodate these changes, low-pass filters for SUS measurements and rates as well as MGM measurements and rates are included. Usage of the new controller as well as settings of the low-pass filters can be handled via parameter commands. +- Simplify and fix the chip and copy protection functions in the core controller. This mechanism + now is always performed for the target chip and target copy in the reboot handlers. ## Added diff --git a/bsp_q7s/boardtest/Q7STestTask.cpp b/bsp_q7s/boardtest/Q7STestTask.cpp index 50a34284..03805fde 100644 --- a/bsp_q7s/boardtest/Q7STestTask.cpp +++ b/bsp_q7s/boardtest/Q7STestTask.cpp @@ -218,15 +218,30 @@ 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, + true); + if (result != returnvalue::OK) { + sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; + } + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_0, xsc::Copy::COPY_1, + true); + if (result != returnvalue::OK) { + sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; + } + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_0, + true); + if (result != returnvalue::OK) { + sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl; + } + result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, + true); 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 +254,8 @@ 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 +268,8 @@ 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 +282,8 @@ 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 e4fb07ec..e1b48fd2 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1232,11 +1232,9 @@ ReturnValue_t CoreController::gracefulShutdownTasks(xsc::Chip chip, xsc::Copy co << std::endl; } - // If any boot copies are unprotected. - // Actually this function only ensures that reboots to the own image are protected.. - result = setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true, protOpPerformed, - false); - if (result == returnvalue::OK and protOpPerformed) { + // Ensure that 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; @@ -1277,144 +1275,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::CHIP_1) { + 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) { @@ -1459,7 +1363,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; @@ -1471,28 +1374,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 7c17de6f..79224ed2 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -199,8 +199,8 @@ 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; @@ -304,12 +304,10 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe Countdown sdCardCheckCd = Countdown(INIT_SD_CARD_CHECK_TIMEOUT); /** - * Index 0: Chip 0 Copy 0 - * Index 1: Chip 0 Copy 1 - * Index 2: Chip 1 Copy 0 - * Index 3: Chip 1 Copy 1 + * First index: Chip. + * Second index: Copy. */ - std::array protArray; + bool protArray[2][2]{}; PeriodicOperationDivider opDivider5; PeriodicOperationDivider opDivider10; @@ -375,9 +373,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe ReturnValue_t handleProtInfoUpdateLine(std::string nextLine); ReturnValue_t handleSwitchingSdCardsOffNonBlocking(); - 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);