diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashDone.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashDone.cpp index bad5d6ca..a5381ebd 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashDone.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashDone.cpp @@ -55,8 +55,8 @@ namespace DebugServer::Gdb::AvrGdb::CommandPackets debugSession.programmingSession.reset(); - Logger::warning("Program memory updated"); targetControllerService.disableProgrammingMode(); + Logger::warning("Program memory updated"); Logger::warning("Resetting target"); targetControllerService.resetTarget(); diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashErase.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashErase.cpp index 0d5afc3b..02b5e069 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashErase.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashErase.cpp @@ -54,8 +54,6 @@ namespace DebugServer::Gdb::AvrGdb::CommandPackets try { targetControllerService.enableProgrammingMode(); - Logger::warning("Erasing program memory, in preparation for programming"); - // We don't erase a specific address range - we just erase the entire program memory. targetControllerService.eraseMemory( gdbTargetDescriptor.programAddressSpaceDescriptor, diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashWrite.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashWrite.cpp index 78414bd3..b00fabdd 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashWrite.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/FlashWrite.cpp @@ -55,32 +55,33 @@ namespace DebugServer::Gdb::AvrGdb::CommandPackets if (!debugSession.programmingSession.has_value()) { debugSession.programmingSession = ProgrammingSession{this->startAddress, this->buffer}; + debugSession.connection.writePacket(OkResponsePacket{}); + return; + } - } else { - auto& programmingSession = debugSession.programmingSession.value(); - const auto currentEndAddress = programmingSession.startAddress + programmingSession.buffer.size() - 1; - const auto expectedStartAddress = (currentEndAddress + 1); + auto& programmingSession = debugSession.programmingSession.value(); + const auto currentEndAddress = programmingSession.startAddress + programmingSession.buffer.size() - 1; + const auto expectedStartAddress = (currentEndAddress + 1); - if (this->startAddress < expectedStartAddress) { - throw Exception{"Invalid start address from GDB - the buffer would overlap a previous buffer"}; - } - - if (this->startAddress > expectedStartAddress) { - // There is a gap in the buffer sent by GDB. Fill it with 0xFF - programmingSession.buffer.insert( - programmingSession.buffer.end(), - this->startAddress - expectedStartAddress, - 0xFF - ); - } + if (this->startAddress < expectedStartAddress) { + throw Exception{"Invalid start address from GDB - the buffer would overlap a previous buffer"}; + } + if (this->startAddress > expectedStartAddress) { + // There is a gap in the buffer sent by GDB. Fill it with 0xFF programmingSession.buffer.insert( programmingSession.buffer.end(), - this->buffer.begin(), - this->buffer.end() + this->startAddress - expectedStartAddress, + 0xFF ); } + programmingSession.buffer.insert( + programmingSession.buffer.end(), + this->buffer.begin(), + this->buffer.end() + ); + debugSession.connection.writePacket(OkResponsePacket{}); } catch (const Exception& exception) { diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashDone.cpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashDone.cpp index 2c51dcdc..5759a287 100644 --- a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashDone.cpp +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashDone.cpp @@ -80,8 +80,8 @@ namespace DebugServer::Gdb::RiscVGdb::CommandPackets debugSession.programmingSession.reset(); - Logger::warning("Program memory updated"); targetControllerService.disableProgrammingMode(); + Logger::warning("Program memory updated"); Logger::warning("Resetting target"); targetControllerService.resetTarget(); diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashErase.cpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashErase.cpp index 3f80233d..c81f1747 100644 --- a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashErase.cpp +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashErase.cpp @@ -59,7 +59,7 @@ namespace DebugServer::Gdb::RiscVGdb::CommandPackets throw Exception{"Memory segment (\"" + segmentDescriptor.name + "\") not writable in programming mode"}; } - Logger::warning("Erasing \"" + segmentDescriptor.name + "\" segment, in preparation for programming"); + Logger::debug("Erase segment key: `" + segmentDescriptor.key + "`"); targetControllerService.enableProgrammingMode(); diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashWrite.cpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashWrite.cpp index e52a586c..71ef578a 100644 --- a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashWrite.cpp +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/FlashWrite.cpp @@ -55,32 +55,33 @@ namespace DebugServer::Gdb::RiscVGdb::CommandPackets if (!debugSession.programmingSession.has_value()) { debugSession.programmingSession = ProgrammingSession{this->startAddress, this->buffer}; + debugSession.connection.writePacket(OkResponsePacket{}); + return; + } - } else { - auto& programmingSession = debugSession.programmingSession.value(); - const auto currentEndAddress = programmingSession.startAddress + programmingSession.buffer.size() - 1; - const auto expectedStartAddress = (currentEndAddress + 1); + auto& programmingSession = debugSession.programmingSession.value(); + const auto currentEndAddress = programmingSession.startAddress + programmingSession.buffer.size() - 1; + const auto expectedStartAddress = (currentEndAddress + 1); - if (this->startAddress < expectedStartAddress) { - throw Exception{"Invalid start address from GDB - the buffer would overlap a previous buffer"}; - } - - if (this->startAddress > expectedStartAddress) { - // There is a gap in the buffer sent by GDB. Fill it with 0xFF - programmingSession.buffer.insert( - programmingSession.buffer.end(), - this->startAddress - expectedStartAddress, - 0xFF - ); - } + if (this->startAddress < expectedStartAddress) { + throw Exception{"Invalid start address from GDB - the buffer would overlap a previous buffer"}; + } + if (this->startAddress > expectedStartAddress) { + // There is a gap in the buffer sent by GDB. Fill it with 0xFF programmingSession.buffer.insert( programmingSession.buffer.end(), - this->buffer.begin(), - this->buffer.end() + this->startAddress - expectedStartAddress, + 0xFF ); } + programmingSession.buffer.insert( + programmingSession.buffer.end(), + this->buffer.begin(), + this->buffer.end() + ); + debugSession.connection.writePacket(OkResponsePacket{}); } catch (const Exception& exception) { diff --git a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp index a9fa052e..5f389e57 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp @@ -397,6 +397,15 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } } + void EdbgAvr8Interface::clearAllBreakpoints() { + this->clearAllSoftwareBreakpoints(); + + // Clear all hardware breakpoints + while (!this->hardwareBreakpointNumbersByAddress.empty()) { + this->clearHardwareBreakpoint(this->hardwareBreakpointNumbersByAddress.begin()->first); + } + } + TargetRegisterDescriptorAndValuePairs EdbgAvr8Interface::readRegisters( const Targets::TargetRegisterDescriptors& descriptors ) { @@ -868,27 +877,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr return; } - /* - * When clearing individual software breakpoints via EDBG tools, the breakpoints are not actually removed - * until this next program flow command. - * - * This wouldn't be a problem, if the tool handled the stale breakpoint removals properly, when programming - * the target (overwriting the program memory at which the software breakpoints reside). But the tool - * doesn't handle this properly - it seems to completely ignore the fact that the program memory has been - * updated, and so it will corrupt the new program by overwriting the program memory where the old - * software breakpoints used to reside. The memory is overwritten with the old instruction - the one that - * was captured at the time the software breakpoint was inserted. So we end up with a corrupted program. - * - * To avoid this issue, we send the 'clear all software breakpoints' command to the tool, just before - * entering programming mode. That command will clear all breakpoints immediately, preventing program - * memory corruption at the next flow control command. - * - * The TargetController will reinsert all breakpoints at the end of a programming session, so the breakpoints - * that we clear here will be restored. - */ - Logger::debug("Clearing all software breakpoints in preparation for programming mode"); - this->clearAllSoftwareBreakpoints(); - const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( EnterProgrammingMode{} ); @@ -1312,15 +1300,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr this->pendingSoftwareBreakpointDeletions.clear(); } - void EdbgAvr8Interface::clearAllBreakpoints() { - this->clearAllSoftwareBreakpoints(); - - // Clear all hardware breakpoints - while (!this->hardwareBreakpointNumbersByAddress.empty()) { - this->clearHardwareBreakpoint(this->hardwareBreakpointNumbersByAddress.begin()->first); - } - } - void EdbgAvr8Interface::injectActiveBreakpoints( const TargetAddressSpaceDescriptor& addressSpaceDescriptor, const TargetMemorySegmentDescriptor& memorySegmentDescriptor, diff --git a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp index f96af426..ddff81f5 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp @@ -113,8 +113,9 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr Targets::Microchip::Avr8::TargetSignature getDeviceId() override; - virtual void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; - virtual void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; + void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; + void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; + void clearAllBreakpoints() override; Targets::TargetRegisterDescriptorAndValuePairs readRegisters( const Targets::TargetRegisterDescriptors& descriptors @@ -247,7 +248,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr void setHardwareBreakpoint(Targets::TargetMemoryAddress address); void clearHardwareBreakpoint(Targets::TargetMemoryAddress address); void clearAllSoftwareBreakpoints(); - void clearAllBreakpoints(); void injectActiveBreakpoints( const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, diff --git a/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp index c15488bd..a40c34e6 100644 --- a/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp @@ -99,6 +99,7 @@ namespace DebugToolDrivers::TargetInterfaces::Microchip::Avr8 virtual void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) = 0; virtual void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) = 0; + virtual void clearAllBreakpoints() = 0; virtual Targets::TargetMemoryAddress getProgramCounter() = 0; virtual void setProgramCounter(Targets::TargetMemoryAddress programCounter) = 0; diff --git a/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp b/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp index bb6ba845..62570de1 100644 --- a/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp +++ b/src/DebugToolDrivers/Wch/Protocols/WchLink/WchLinkInterface.cpp @@ -21,8 +21,6 @@ #include "src/Helpers/BiMap.hpp" #include "src/Services/StringService.hpp" -#include "src/Logger/Logger.hpp" - #include "src/TargetController/Exceptions/DeviceCommunicationFailure.hpp" namespace DebugToolDrivers::Wch::Protocols::WchLink diff --git a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp index b6ebb707..b6fc4c21 100644 --- a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp +++ b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.cpp @@ -33,7 +33,9 @@ namespace DebugToolDrivers::Wch using ::Targets::TargetAddressSpaceDescriptor; using ::Targets::TargetMemorySegmentDescriptor; using ::Targets::TargetProgramBreakpoint; + using ::Targets::BreakpointResources; using ::Targets::TargetMemorySegmentType; + using ::Targets::TargetRegisterDescriptor; using ::Targets::TargetRegisterDescriptors; using ::Targets::TargetRegisterDescriptorAndValuePairs; @@ -62,6 +64,7 @@ namespace DebugToolDrivers::Wch , toolInfo(toolInfo) , sysAddressSpaceDescriptor(this->targetDescriptionFile.getSystemAddressSpaceDescriptor()) , mainProgramSegmentDescriptor(this->sysAddressSpaceDescriptor.getMemorySegmentDescriptor("main_program")) + , bootProgramSegmentDescriptor(this->sysAddressSpaceDescriptor.getMemorySegmentDescriptor("boot_program")) , flashProgramOpcodes( WchLinkDebugInterface::getFlashProgramOpcodes( this->targetDescriptionFile.getProperty("wch_link_interface", "programming_opcode_key").value @@ -129,7 +132,6 @@ namespace DebugToolDrivers::Wch } void WchLinkDebugInterface::deactivate() { - this->riscVTranslator.clearAllTriggers(); this->riscVTranslator.deactivate(); const auto response = this->wchLinkInterface.sendCommandAndWaitForResponse(Commands::Control::DetachTarget{}); @@ -142,7 +144,7 @@ namespace DebugToolDrivers::Wch return "0x" + Services::StringService::toHex(this->cachedVariantId.value()); } - Targets::TargetExecutionState WchLinkDebugInterface::getExecutionState() { + TargetExecutionState WchLinkDebugInterface::getExecutionState() { return this->riscVTranslator.getExecutionState(); } @@ -162,7 +164,7 @@ namespace DebugToolDrivers::Wch this->riscVTranslator.reset(); } - Targets::BreakpointResources WchLinkDebugInterface::getBreakpointResources() { + BreakpointResources WchLinkDebugInterface::getBreakpointResources() { return { .hardwareBreakpoints = this->riscVTranslator.getTriggerCount(), .softwareBreakpoints = 0xFFFFFFFF, // TODO: Use the program memory size to determine the limit. @@ -187,22 +189,22 @@ namespace DebugToolDrivers::Wch } } - Targets::TargetRegisterDescriptorAndValuePairs WchLinkDebugInterface::readCpuRegisters( - const Targets::TargetRegisterDescriptors& descriptors + TargetRegisterDescriptorAndValuePairs WchLinkDebugInterface::readCpuRegisters( + const TargetRegisterDescriptors& descriptors ) { return this->riscVTranslator.readCpuRegisters(descriptors); } - void WchLinkDebugInterface::writeCpuRegisters(const Targets::TargetRegisterDescriptorAndValuePairs& registers) { + void WchLinkDebugInterface::writeCpuRegisters(const TargetRegisterDescriptorAndValuePairs& registers) { return this->riscVTranslator.writeCpuRegisters(registers); } - Targets::TargetMemoryBuffer WchLinkDebugInterface::readMemory( + TargetMemoryBuffer WchLinkDebugInterface::readMemory( const TargetAddressSpaceDescriptor& addressSpaceDescriptor, const TargetMemorySegmentDescriptor& memorySegmentDescriptor, - Targets::TargetMemoryAddress startAddress, - Targets::TargetMemorySize bytes, - const std::set& excludedAddressRanges + TargetMemoryAddress startAddress, + TargetMemorySize bytes, + const std::set& excludedAddressRanges ) { return this->riscVTranslator.readMemory( addressSpaceDescriptor, @@ -216,8 +218,8 @@ namespace DebugToolDrivers::Wch void WchLinkDebugInterface::writeMemory( const TargetAddressSpaceDescriptor& addressSpaceDescriptor, const TargetMemorySegmentDescriptor& memorySegmentDescriptor, - Targets::TargetMemoryAddress startAddress, - Targets::TargetMemoryBufferSpan buffer + TargetMemoryAddress startAddress, + TargetMemoryBufferSpan buffer ) { if (memorySegmentDescriptor.type == TargetMemorySegmentType::FLASH) { /* @@ -293,8 +295,8 @@ namespace DebugToolDrivers::Wch * The erase operation will silently fail, and the target will be left in a bad state, causing the subsequent * full block write to corrupt the target's program memory * - * - The reset is what causes the erase operation to fail. But I have no idea why. I've inspected the relevant - * registers, at the relevant times, but have found nothing significant that could explain this + * - The reset is what causes the erase operation to fail, but I have no idea why. I've inspected the relevant + * registers, at the relevant times, but found nothing significant that could explain this * * - Subsequent resets do not fix the issue, but another full block write does. My guess is that the first full * block write (which corrupted program memory) corrected the target state, cleaning the mess made by the @@ -304,11 +306,11 @@ namespace DebugToolDrivers::Wch } void WchLinkDebugInterface::enableProgrammingMode() { + // TODO: Move this to target driver. After v2.0.0. + this->clearAllBreakpoints(); } - void WchLinkDebugInterface::disableProgrammingMode() { - this->softwareBreakpointRegistry.clear(); - } + void WchLinkDebugInterface::disableProgrammingMode() {} void WchLinkDebugInterface::applyAccessRestrictions(TargetMemorySegmentDescriptor& memorySegmentDescriptor) { if (memorySegmentDescriptor.type == TargetMemorySegmentType::FLASH) { @@ -320,7 +322,7 @@ namespace DebugToolDrivers::Wch } } - void WchLinkDebugInterface::applyAccessRestrictions(Targets::TargetRegisterDescriptor& registerDescriptor) { + void WchLinkDebugInterface::applyAccessRestrictions(TargetRegisterDescriptor& registerDescriptor) { // I don't believe any further access restrictions are required for registers. TODO: Review after v2.0.0. } @@ -329,23 +331,6 @@ namespace DebugToolDrivers::Wch throw Exception{"Invalid software breakpoint size (" + std::to_string(breakpoint.size) + ")"}; } - const auto originalData = this->readMemory( - breakpoint.addressSpaceDescriptor, - breakpoint.memorySegmentDescriptor, - breakpoint.address, - breakpoint.size, - {} - ); - - const auto softwareBreakpoint = ::Targets::RiscV::ProgramBreakpoint{ - breakpoint, - static_cast<::Targets::RiscV::Opcodes::Opcode>( - breakpoint.size == 2 - ? (originalData[1] << 8) | originalData[0] - : (originalData[3] << 24) | (originalData[2] << 16) | (originalData[1] << 8) | originalData[0] - ) - }; - static constexpr auto EBREAK_OPCODE = std::to_array({ static_cast(::Targets::RiscV::Opcodes::Ebreak), static_cast(::Targets::RiscV::Opcodes::Ebreak >> 8), @@ -359,15 +344,15 @@ namespace DebugToolDrivers::Wch }); this->writeMemory( - softwareBreakpoint.addressSpaceDescriptor, - softwareBreakpoint.memorySegmentDescriptor, - softwareBreakpoint.address, - softwareBreakpoint.size == 2 + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size == 2 ? TargetMemoryBufferSpan{COMPRESSED_EBREAK_OPCODE} : TargetMemoryBufferSpan{EBREAK_OPCODE} ); - this->softwareBreakpointRegistry.insert(softwareBreakpoint); + this->softwareBreakpointRegistry.insert(breakpoint); } void WchLinkDebugInterface::clearSoftwareBreakpoint(const TargetProgramBreakpoint& breakpoint) { @@ -375,37 +360,26 @@ namespace DebugToolDrivers::Wch throw Exception{"Invalid software breakpoint size (" + std::to_string(breakpoint.size) + ")"}; } - const auto softwareBreakpointOpt = this->softwareBreakpointRegistry.find(breakpoint); - if (!softwareBreakpointOpt.has_value()) { - throw TargetOperationFailure{ - "Unknown software breakpoint (byte address: 0x" + Services::StringService::toHex(breakpoint.address) - + ")" - }; - } - - const auto& softwareBreakpoint = softwareBreakpointOpt->get(); - if (!softwareBreakpoint.originalInstruction.has_value()) { - throw InternalFatalErrorException{"Missing original opcode"}; - } - this->writeMemory( - softwareBreakpoint.addressSpaceDescriptor, - softwareBreakpoint.memorySegmentDescriptor, - softwareBreakpoint.address, - softwareBreakpoint.size == 2 - ? TargetMemoryBuffer{ - static_cast(*(softwareBreakpoint.originalInstruction)), - static_cast(*(softwareBreakpoint.originalInstruction) >> 8) - } - : TargetMemoryBuffer{ - static_cast(*(softwareBreakpoint.originalInstruction)), - static_cast(*(softwareBreakpoint.originalInstruction) >> 8), - static_cast(*(softwareBreakpoint.originalInstruction) >> 16), - static_cast(*(softwareBreakpoint.originalInstruction) >> 24) - } + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + TargetMemoryBufferSpan{ + breakpoint.originalData.begin(), + breakpoint.originalData.begin() + breakpoint.size + } ); - this->softwareBreakpointRegistry.remove(softwareBreakpoint); + this->softwareBreakpointRegistry.remove(breakpoint); + } + + void WchLinkDebugInterface::clearAllBreakpoints() { + this->riscVTranslator.clearAllTriggers(); + for (const auto [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->clearSoftwareBreakpoint(breakpoint); + } + } } void WchLinkDebugInterface::writeProgramMemoryPartialBlock( diff --git a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp index 91b8b704..6086adb0 100644 --- a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp +++ b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp @@ -17,7 +17,6 @@ #include "src/Targets/ProgramBreakpointRegistry.hpp" #include "src/Targets/RiscV/Wch/TargetDescriptionFile.hpp" -#include "src/Targets/RiscV/ProgramBreakpoint.hpp" namespace DebugToolDrivers::Wch { @@ -96,6 +95,7 @@ namespace DebugToolDrivers::Wch const Targets::TargetAddressSpaceDescriptor sysAddressSpaceDescriptor; const Targets::TargetMemorySegmentDescriptor& mainProgramSegmentDescriptor; + const Targets::TargetMemorySegmentDescriptor& bootProgramSegmentDescriptor; /** * The 'target activation' command returns a payload of 5 bytes. @@ -107,13 +107,14 @@ namespace DebugToolDrivers::Wch std::optional cachedVariantId; std::optional cachedTargetId; - Targets::ProgramBreakpointRegistryGeneric softwareBreakpointRegistry; + Targets::ProgramBreakpointRegistry softwareBreakpointRegistry; std::span flashProgramOpcodes; Targets::TargetMemorySize programmingBlockSize; void setSoftwareBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint); void clearSoftwareBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint); + void clearAllBreakpoints(); void writeProgramMemoryPartialBlock( const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, diff --git a/src/ProjectConfig.cpp b/src/ProjectConfig.cpp index f3ded5e5..5d2493ab 100644 --- a/src/ProjectConfig.cpp +++ b/src/ProjectConfig.cpp @@ -213,6 +213,10 @@ TargetConfig::TargetConfig(const YAML::Node& targetNode) { this->programMemoryCache = targetNode["program_memory_cache"].as(this->programMemoryCache); } + if (targetNode["delta_programming"]) { + this->deltaProgramming = targetNode["delta_programming"].as(this->deltaProgramming); + } + if (targetNode["reserve_stepping_breakpoint"]) { this->reserveSteppingBreakpoint = targetNode["reserve_stepping_breakpoint"].as(false); } diff --git a/src/ProjectConfig.hpp b/src/ProjectConfig.hpp index fd57d079..aa54cd7a 100644 --- a/src/ProjectConfig.hpp +++ b/src/ProjectConfig.hpp @@ -56,6 +56,13 @@ struct TargetConfig */ bool programMemoryCache = true; + /** + * Determines whether Bloom will employ "delta programming" during programming sessions. + * + * Not all targets support delta programming. + */ + bool deltaProgramming = true; + /** * Determines if Bloom will reserve a single hardware breakpoint for stepping operations. */ diff --git a/src/Services/TargetControllerService.cpp b/src/Services/TargetControllerService.cpp index e666ba9f..cdb1b2bf 100644 --- a/src/Services/TargetControllerService.cpp +++ b/src/Services/TargetControllerService.cpp @@ -220,7 +220,7 @@ namespace Services addressSpaceDescriptor, memorySegmentDescriptor, startAddress, - buffer + TargetMemoryBuffer{buffer.begin(), buffer.end()} ), this->defaultTimeout, this->activeAtomicSessionId diff --git a/src/Services/TargetControllerService.hpp b/src/Services/TargetControllerService.hpp index e7e434a5..33f92244 100644 --- a/src/Services/TargetControllerService.hpp +++ b/src/Services/TargetControllerService.hpp @@ -288,7 +288,7 @@ namespace Services std::optional activeAtomicSessionId = std::nullopt; - std::chrono::milliseconds defaultTimeout = std::chrono::milliseconds{30000}; + std::chrono::milliseconds defaultTimeout = std::chrono::milliseconds{90000}; TargetController::AtomicSessionIdType startAtomicSession(); void endAtomicSession(TargetController::AtomicSessionIdType sessionId); diff --git a/src/TargetController/Commands/WriteTargetMemory.hpp b/src/TargetController/Commands/WriteTargetMemory.hpp index c1e50a23..40fc5622 100644 --- a/src/TargetController/Commands/WriteTargetMemory.hpp +++ b/src/TargetController/Commands/WriteTargetMemory.hpp @@ -18,18 +18,18 @@ namespace TargetController::Commands const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor; const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor; Targets::TargetMemoryAddress startAddress; - Targets::TargetMemoryBufferSpan buffer; + Targets::TargetMemoryBuffer buffer; WriteTargetMemory( const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, Targets::TargetMemoryAddress startAddress, - Targets::TargetMemoryBufferSpan buffer + Targets::TargetMemoryBuffer&& buffer ) : addressSpaceDescriptor(addressSpaceDescriptor) , memorySegmentDescriptor(memorySegmentDescriptor) , startAddress(startAddress) - , buffer(buffer) + , buffer(std::move(buffer)) {}; [[nodiscard]] CommandType getType() const override { diff --git a/src/TargetController/TargetControllerComponent.cpp b/src/TargetController/TargetControllerComponent.cpp index 18b12039..00849c56 100644 --- a/src/TargetController/TargetControllerComponent.cpp +++ b/src/TargetController/TargetControllerComponent.cpp @@ -13,6 +13,7 @@ #include "src/Services/PathService.hpp" #include "src/Services/ProcessService.hpp" #include "src/Services/StringService.hpp" +#include "src/Services/AlignmentService.hpp" #include "src/Logger/Logger.hpp" #include "Exceptions/TargetOperationFailure.hpp" @@ -287,9 +288,34 @@ namespace TargetController this->acquireHardware(); - if (this->targetState->executionState != TargetExecutionState::RUNNING) { -// this->target->run(); -// this->targetState->executionState = TargetExecutionState::RUNNING; + this->targetState = std::make_unique( + TargetExecutionState::UNKNOWN, + TargetMode::DEBUGGING, + std::nullopt + ); + this->refreshExecutionState(); + + if (this->environmentConfig.targetConfig.hardwareBreakpoints) { + const auto& breakpointResources = this->targetDescriptor->breakpointResources; + Logger::info("Available hardware breakpoints: " + std::to_string(breakpointResources.hardwareBreakpoints)); + Logger::info( + "Reserved hardware breakpoints: " + std::to_string(breakpointResources.reservedHardwareBreakpoints) + ); + + } else { + Logger::warning("Hardware breakpoints have been disabled"); + } + + if ( + this->targetState->executionState == TargetExecutionState::STOPPED + && this->environmentConfig.targetConfig.resumeOnStartup + ) { + try { + this->resumeTarget(); + + } catch (const Exceptions::TargetOperationFailure& exception) { + Logger::error("Failed to resume target execution on startup - error: " + exception.getMessage()); + } } this->state = TargetControllerState::ACTIVE; @@ -569,35 +595,7 @@ namespace TargetController this->target->postActivate(); - this->targetState = std::make_unique( - TargetExecutionState::UNKNOWN, - TargetMode::DEBUGGING, - std::nullopt - ); - this->refreshExecutionState(); - - if (this->environmentConfig.targetConfig.hardwareBreakpoints) { - const auto& breakpointResources = this->targetDescriptor->breakpointResources; - Logger::info("Available hardware breakpoints: " + std::to_string(breakpointResources.hardwareBreakpoints)); - Logger::info( - "Reserved hardware breakpoints: " + std::to_string(breakpointResources.reservedHardwareBreakpoints) - ); - - } else { - Logger::warning("Hardware breakpoints have been disabled"); - } - - if ( - this->targetState->executionState == TargetExecutionState::STOPPED - && this->environmentConfig.targetConfig.resumeOnStartup - ) { - try { - this->resumeTarget(); - - } catch (const Exceptions::TargetOperationFailure& exception) { - Logger::error("Failed to resume target execution on startup - error: " + exception.getMessage()); - } - } + this->deltaProgrammingInterface = this->target->deltaProgrammingInterface(); } void TargetControllerComponent::releaseHardware() { @@ -752,7 +750,8 @@ namespace TargetController ); } - return cache.fetch(startAddress, bytes); + const auto cachedData = cache.fetch(startAddress, bytes); + return TargetMemoryBuffer{cachedData.begin(), cachedData.end()}; } return this->target->readMemory( @@ -783,7 +782,13 @@ namespace TargetController this->target->writeMemory(addressSpaceDescriptor, memorySegmentDescriptor, startAddress, buffer); - if (isProgramMemory && this->environmentConfig.targetConfig.programMemoryCache) { + if ( + isProgramMemory + && ( + this->environmentConfig.targetConfig.programMemoryCache + || this->environmentConfig.targetConfig.deltaProgramming + ) + ) { this->getProgramMemoryCache(memorySegmentDescriptor).insert(startAddress, buffer); } @@ -811,7 +816,10 @@ namespace TargetController throw Exception{"Cannot erase program memory - programming mode not enabled."}; } - if (this->environmentConfig.targetConfig.programMemoryCache) { + if ( + this->environmentConfig.targetConfig.programMemoryCache + || this->environmentConfig.targetConfig.deltaProgramming + ) { Logger::debug("Clearing program memory cache"); this->getProgramMemoryCache(memorySegmentDescriptor).clear(); } @@ -847,7 +855,10 @@ namespace TargetController if ( breakpoint.type == TargetProgramBreakpoint::Type::SOFTWARE - && this->environmentConfig.targetConfig.programMemoryCache + && ( + this->environmentConfig.targetConfig.programMemoryCache + || this->environmentConfig.targetConfig.deltaProgramming + ) && this->target->isProgramMemory( breakpoint.addressSpaceDescriptor, breakpoint.memorySegmentDescriptor, @@ -881,39 +892,43 @@ namespace TargetController Logger::debug("Removing breakpoint at byte address 0x" + StringService::toHex(breakpoint.address)); - if (!registry.contains(breakpoint)) { + + const auto registeredBreakpointOpt = registry.find(breakpoint); + if (!registeredBreakpointOpt.has_value()) { Logger::debug("Breakpoint not found in registry - ignoring removal request"); return; } - this->target->removeProgramBreakpoint(breakpoint); - registry.remove(breakpoint); + const auto& registeredBreakpoint = registeredBreakpointOpt->get(); + this->target->removeProgramBreakpoint(registeredBreakpoint); if ( - breakpoint.type == TargetProgramBreakpoint::Type::SOFTWARE - && this->environmentConfig.targetConfig.programMemoryCache + registeredBreakpoint.type == TargetProgramBreakpoint::Type::SOFTWARE + && ( + this->environmentConfig.targetConfig.programMemoryCache + || this->environmentConfig.targetConfig.deltaProgramming + ) && this->target->isProgramMemory( - breakpoint.addressSpaceDescriptor, - breakpoint.memorySegmentDescriptor, - breakpoint.address, - breakpoint.size + registeredBreakpoint.addressSpaceDescriptor, + registeredBreakpoint.memorySegmentDescriptor, + registeredBreakpoint.address, + registeredBreakpoint.size ) ) { - auto& cache = this->getProgramMemoryCache(breakpoint.memorySegmentDescriptor); - if (cache.contains(breakpoint.address, breakpoint.size)) { - // Update program memory cache + auto& cache = this->getProgramMemoryCache(registeredBreakpoint.memorySegmentDescriptor); + if (cache.contains(registeredBreakpoint.address, registeredBreakpoint.size)) { + // Update program memory cache with the original instruction cache.insert( - breakpoint.address, - this->target->readMemory( - breakpoint.addressSpaceDescriptor, - breakpoint.memorySegmentDescriptor, - breakpoint.address, - breakpoint.size, - {} - ) + registeredBreakpoint.address, + TargetMemoryBufferSpan{ + registeredBreakpoint.originalData.begin(), + registeredBreakpoint.originalData.begin() + registeredBreakpoint.size + } ); } } + + registry.remove(registeredBreakpoint); } void TargetControllerComponent::clearAllBreakpoints() { @@ -935,12 +950,21 @@ namespace TargetController this->target->enableProgrammingMode(); Logger::warning("Programming mode enabled"); + if (this->environmentConfig.targetConfig.deltaProgramming && this->deltaProgrammingInterface != nullptr) { + this->deltaProgrammingSession = DeltaProgramming::Session{}; + } + auto newState = *(this->targetState); newState.mode = TargetMode::PROGRAMMING; this->updateTargetState(newState); } void TargetControllerComponent::disableProgrammingMode() { + if (this->deltaProgrammingSession.has_value()) { + this->commitDeltaProgrammingSession(*(this->deltaProgrammingSession)); + this->deltaProgrammingSession = std::nullopt; + } + Logger::debug("Disabling programming mode"); this->target->disableProgrammingMode(); Logger::info("Programming mode disabled"); @@ -948,14 +972,41 @@ namespace TargetController Logger::info("Restoring breakpoints"); this->target->stop(); - for (const auto& [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) { - for (const auto& [address, breakpoint] : breakpointsByAddress) { + static const auto refreshOriginalData = [this] (TargetProgramBreakpoint& breakpoint) { + const auto originalData = this->readTargetMemory( + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size, + {}, + true + ); + + std::copy(originalData.begin(), originalData.end(), breakpoint.originalData.begin()); + }; + + for (auto& [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) { + for (auto& [address, breakpoint] : breakpointsByAddress) { + refreshOriginalData(breakpoint); this->target->setProgramBreakpoint(breakpoint); + + auto& cache = this->getProgramMemoryCache(breakpoint.memorySegmentDescriptor); + cache.insert( + breakpoint.address, + this->target->readMemory( + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size, + {} + ) + ); } } - for (const auto& [addressSpaceId, breakpointsByAddress] : this->hardwareBreakpointRegistry) { - for (const auto& [address, breakpoint] : breakpointsByAddress) { + for (auto& [addressSpaceId, breakpointsByAddress] : this->hardwareBreakpointRegistry) { + for (auto& [address, breakpoint] : breakpointsByAddress) { + refreshOriginalData(breakpoint); this->target->setProgramBreakpoint(breakpoint); } } @@ -981,6 +1032,163 @@ namespace TargetController return cacheIt->second; } + void TargetControllerComponent::commitDeltaProgrammingSession(const DeltaProgramming::Session& session) { + using Services::AlignmentService; + using Services::StringService; + + /* + * If a single write operation cannot be committed, we must abandon the whole session. + * + * We prepare CommitOperation objects and then commit all of them at the end, allowing for the abandoning of + * the session, if necessary. + */ + struct CommitOperation + { + const TargetAddressSpaceDescriptor& addressSpaceDescriptor; + const TargetMemorySegmentDescriptor& memorySegmentDescriptor; + std::vector deltaSegments; + }; + auto commitOperations = std::vector{}; + + for (const auto& [segmentId, writeOperation] : session.writeOperationsBySegmentId) { + auto& segmentCache = this->getProgramMemoryCache(writeOperation.memorySegmentDescriptor); + + // Can the program memory cache facilitate diffing with all regions in this write operation? + for (const auto& region : writeOperation.regions) { + if (!segmentCache.contains(region.addressRange.startAddress, region.addressRange.size())) { + Logger::info("Abandoning delta programming session - insufficient data in program memory cache"); + return this->abandonDeltaProgrammingSession(session); + } + } + + const auto alignTo = this->deltaProgrammingInterface->deltaBlockSize( + writeOperation.addressSpaceDescriptor, + writeOperation.memorySegmentDescriptor + ); + + /* + * Ensure that the segment cache has sufficient data to facilitate delta segment alignment + * + * If the cache doesn't contain the necessary data for alignment, we just fill it with 0xFF instead of + * obtaining the data with a read operation. This saves us some time. + */ + for (const auto& region : writeOperation.regions) { + const auto alignedAddress = AlignmentService::alignMemoryAddress( + region.addressRange.startAddress, + alignTo + ); + const auto alignedSize = AlignmentService::alignMemorySize( + region.addressRange.size() + (region.addressRange.startAddress - alignedAddress), + alignTo + ); + + if (!segmentCache.contains(alignedAddress, alignedSize)) { + if (region.addressRange.startAddress != alignedAddress) { + segmentCache.fill(alignedAddress, region.addressRange.startAddress - alignedAddress, 0xFF); + } + + if (region.addressRange.size() != alignedSize) { + segmentCache.fill( + region.addressRange.endAddress + 1, + alignedSize - region.addressRange.size(), + 0xFF + ); + } + } + } + + /* + * When constructing the delta segments, any software breakpoints currently installed in this memory + * segment can interfere with the diffing of the new program and the program memory cache. + * + * For this reason, we make a copy of the program memory cache and strip any software breakpoints from it, + * before constructing the delta segments. + */ + auto cacheData = segmentCache.data; + for (const auto& [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + if (breakpoint.memorySegmentDescriptor != writeOperation.memorySegmentDescriptor) { + continue; + } + + std::copy( + breakpoint.originalData.begin(), + breakpoint.originalData.begin() + breakpoint.size, + cacheData.begin() + (breakpoint.address + - breakpoint.memorySegmentDescriptor.addressRange.startAddress) + ); + } + } + + auto operation = CommitOperation{ + .addressSpaceDescriptor = writeOperation.addressSpaceDescriptor, + .memorySegmentDescriptor = writeOperation.memorySegmentDescriptor, + .deltaSegments = writeOperation.deltaSegments(cacheData, alignTo) + }; + + if (operation.deltaSegments.empty()) { + Logger::warning("Abandoning delta programming session - zero delta segments"); + return this->abandonDeltaProgrammingSession(session); + } + + if ( + this->deltaProgrammingInterface->shouldAbandonSession( + operation.addressSpaceDescriptor, + operation.memorySegmentDescriptor, + operation.deltaSegments + ) + ) { + Logger::info("Abandoning delta programming session - upon target driver request"); + return this->abandonDeltaProgrammingSession(session); + } + + commitOperations.emplace_back(std::move(operation)); + } + + Logger::info("Committing delta programming session"); + + for (const auto& operation : commitOperations) { + auto& segmentCache = this->getProgramMemoryCache(operation.memorySegmentDescriptor); + + Logger::info( + std::to_string(operation.deltaSegments.size()) + " delta segment(s) to be flushed to `" + + operation.memorySegmentDescriptor.key + "`" + ); + for (const auto& deltaSegment : operation.deltaSegments) { + Logger::info( + "Flushing delta segment 0x" + StringService::toHex(deltaSegment.addressRange.startAddress) + + " -> 0x" + StringService::toHex(deltaSegment.addressRange.endAddress) + " - " + + std::to_string(deltaSegment.buffer.size()) + " byte(s)" + ); + this->target->writeMemory( + operation.addressSpaceDescriptor, + operation.memorySegmentDescriptor, + deltaSegment.addressRange.startAddress, + deltaSegment.buffer + ); + + segmentCache.insert(deltaSegment.addressRange.startAddress, deltaSegment.buffer); + } + } + } + + void TargetControllerComponent::abandonDeltaProgrammingSession(const DeltaProgramming::Session& session) { + for (const auto& [segmentId, eraseOperation] : session.eraseOperationsBySegmentId) { + this->eraseTargetMemory(eraseOperation.addressSpaceDescriptor, eraseOperation.memorySegmentDescriptor); + } + + for (const auto& [segmentId, writeOperation] : session.writeOperationsBySegmentId) { + for (const auto& region : writeOperation.regions) { + this->writeTargetMemory( + writeOperation.addressSpaceDescriptor, + writeOperation.memorySegmentDescriptor, + region.addressRange.startAddress, + region.buffer + ); + } + } + } + void TargetControllerComponent::onShutdownTargetControllerEvent(const Events::ShutdownTargetController&) { this->shutdown(); } @@ -1105,6 +1313,31 @@ namespace TargetController throw Exception{"Invalid address range"}; } + if ( + this->targetState->mode == TargetMode::PROGRAMMING + && this->deltaProgrammingSession.has_value() + && this->target->isProgramMemory( + command.addressSpaceDescriptor, + command.memorySegmentDescriptor, + command.startAddress, + static_cast(command.buffer.size()) + ) + ) { + Logger::debug( + "Pushing program memory write operation, at 0x" + Services::StringService::toHex(command.startAddress) + + ", " + std::to_string(command.buffer.size()) + " byte(s), to active delta programming session" + ); + + this->deltaProgrammingSession->pushWriteOperation( + command.addressSpaceDescriptor, + command.memorySegmentDescriptor, + command.startAddress, + std::move(command.buffer) + ); + + return std::make_unique(); + } + this->writeTargetMemory( command.addressSpaceDescriptor, command.memorySegmentDescriptor, @@ -1116,6 +1349,26 @@ namespace TargetController } std::unique_ptr TargetControllerComponent::handleEraseTargetMemory(EraseTargetMemory& command) { + if ( + this->targetState->mode == TargetMode::PROGRAMMING + && this->deltaProgrammingSession.has_value() + && this->target->isProgramMemory( + command.addressSpaceDescriptor, + command.memorySegmentDescriptor, + command.memorySegmentDescriptor.addressRange.startAddress, + command.memorySegmentDescriptor.addressRange.size() + ) + ) { + Logger::debug("Pushing program memory erase operation to active delta programming session"); + + this->deltaProgrammingSession->pushEraseOperation( + command.addressSpaceDescriptor, + command.memorySegmentDescriptor + ); + + return std::make_unique(); + } + this->eraseTargetMemory(command.addressSpaceDescriptor, command.memorySegmentDescriptor); return std::make_unique(); } @@ -1128,15 +1381,32 @@ namespace TargetController std::unique_ptr TargetControllerComponent::handleSetProgramBreakpointBreakpointAnyType( SetProgramBreakpointAnyType& command ) { - const auto breakpoint = TargetProgramBreakpoint{ + if (command.size > TargetProgramBreakpoint::MAX_SIZE) { + throw Exception{"Invalid breakpoint size"}; + } + + auto breakpoint = TargetProgramBreakpoint{ .addressSpaceDescriptor = command.addressSpaceDescriptor, .memorySegmentDescriptor = command.memorySegmentDescriptor, .address = command.address, .size = command.size, .type = this->environmentConfig.targetConfig.hardwareBreakpoints && this->availableHardwareBreakpoints() > 0 ? TargetProgramBreakpoint::Type::HARDWARE - : TargetProgramBreakpoint::Type::SOFTWARE + : TargetProgramBreakpoint::Type::SOFTWARE, + .originalData = {} }; + + const auto originalData = this->readTargetMemory( + command.addressSpaceDescriptor, + command.memorySegmentDescriptor, + command.address, + command.size, + {}, + true + ); + + std::copy(originalData.begin(), originalData.end(), breakpoint.originalData.begin()); + this->setProgramBreakpoint(breakpoint); return std::make_unique(breakpoint); } diff --git a/src/TargetController/TargetControllerComponent.hpp b/src/TargetController/TargetControllerComponent.hpp index 5febdd21..1895744a 100644 --- a/src/TargetController/TargetControllerComponent.hpp +++ b/src/TargetController/TargetControllerComponent.hpp @@ -73,6 +73,8 @@ #include "src/Targets/TargetMemorySegmentDescriptor.hpp" #include "src/Targets/ProgramBreakpointRegistry.hpp" #include "src/Targets/TargetMemoryCache.hpp" +#include "src/Targets/DeltaProgramming/DeltaProgrammingInterface.hpp" +#include "src/Targets/DeltaProgramming/Session.hpp" #include "src/EventManager/EventManager.hpp" #include "src/EventManager/EventListener.hpp" @@ -142,6 +144,8 @@ namespace TargetController std::unique_ptr debugTool = nullptr; std::unique_ptr target = nullptr; + Targets::DeltaProgramming::DeltaProgrammingInterface* deltaProgrammingInterface = nullptr; + std::map< Commands::CommandType, std::function(Commands::Command&)> @@ -170,6 +174,11 @@ namespace TargetController */ std::map programMemoryCachesBySegmentId; + /** + * Active delta programming session + */ + std::optional deltaProgrammingSession; + /** * Registers a handler function for a particular command type. * Only one handler function can be registered per command type. @@ -329,6 +338,9 @@ namespace TargetController const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor ); + void commitDeltaProgrammingSession(const Targets::DeltaProgramming::Session& session); + void abandonDeltaProgrammingSession(const Targets::DeltaProgramming::Session& session); + /** * Invokes a shutdown. * diff --git a/src/Targets/CMakeLists.txt b/src/Targets/CMakeLists.txt index f4ebee92..79fe59a5 100755 --- a/src/Targets/CMakeLists.txt +++ b/src/Targets/CMakeLists.txt @@ -17,6 +17,7 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/TargetMemoryCache.cpp ${CMAKE_CURRENT_SOURCE_DIR}/TargetPhysicalInterface.cpp ${CMAKE_CURRENT_SOURCE_DIR}/DynamicRegisterValue.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/DeltaProgramming/Session.cpp ${CMAKE_CURRENT_SOURCE_DIR}/TargetDescription/TargetDescriptionFile.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Microchip/Avr8/Avr8.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Microchip/Avr8/Avr8TargetConfig.cpp diff --git a/src/Targets/DeltaProgramming/DeltaProgrammingInterface.hpp b/src/Targets/DeltaProgramming/DeltaProgrammingInterface.hpp new file mode 100644 index 00000000..f33c4daf --- /dev/null +++ b/src/Targets/DeltaProgramming/DeltaProgrammingInterface.hpp @@ -0,0 +1,36 @@ +#pragma once + +#include "Session.hpp" + +#include "src/Targets/TargetMemory.hpp" +#include "src/Targets/TargetAddressSpaceDescriptor.hpp" +#include "src/Targets/TargetMemorySegmentDescriptor.hpp" + +namespace Targets::DeltaProgramming +{ + class DeltaProgrammingInterface + { + public: + DeltaProgrammingInterface() = default; + virtual ~DeltaProgrammingInterface() = default; + + /** + * The TargetController can align delta segments to a given block size, using the program memory cache. + * This can significantly reduce the number of read operations that would take place for page alignment, + * improving programming speed. + * + * This member function should return the necessary block size for the given memory segment. If alignment is + * not required, it should return a value of 1. + */ + virtual TargetMemorySize deltaBlockSize( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor + ) = 0; + + virtual bool shouldAbandonSession( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + const std::vector& deltaSegments + ) = 0; + }; +} diff --git a/src/Targets/DeltaProgramming/Session.cpp b/src/Targets/DeltaProgramming/Session.cpp new file mode 100644 index 00000000..f60f0b34 --- /dev/null +++ b/src/Targets/DeltaProgramming/Session.cpp @@ -0,0 +1,198 @@ +#include "Session.hpp" + +#include +#include + +#include "src/Services/AlignmentService.hpp" + +namespace Targets::DeltaProgramming +{ + using Targets::TargetAddressSpaceDescriptor; + using Targets::TargetMemorySegmentDescriptor; + using Targets::TargetMemoryAddress; + using Targets::TargetMemoryAddressRange; + using Targets::TargetMemoryBuffer; + + void Session::WriteOperation::Region::mergeWith(const Region& other) { + assert(this->addressRange.intersectsWith(other.addressRange)); + assert(this->addressRange.startAddress <= other.addressRange.startAddress); + + const auto intersectingSize = other.addressRange.intersectingSize(this->addressRange); + if (intersectingSize < addressRange.size()) { + this->buffer.resize(this->buffer.size() + other.buffer.size() - intersectingSize); + this->addressRange.endAddress = static_cast( + this->addressRange.startAddress + this->buffer.size() - 1 + ); + } + + std::copy( + other.buffer.begin(), + other.buffer.end(), + this->buffer.begin() + (other.addressRange.startAddress - this->addressRange.startAddress) + ); + } + + std::vector Session::WriteOperation::deltaSegments( + Targets::TargetMemoryBufferSpan cacheData, + Targets::TargetMemorySize blockSize + ) const { + using Services::AlignmentService; + + // First, we merge any overlapping regions and sort all regions by start address. This simplifies things + auto mergedRegions = std::vector{}; + + for (const auto& region : this->regions) { + for (auto& existingRegion : mergedRegions) { + if (!existingRegion.addressRange.intersectsWith(region.addressRange)) { + continue; + } + + if (region.addressRange.startAddress >= existingRegion.addressRange.startAddress) { + existingRegion.mergeWith(region); + + } else { + auto regionClone = region; + regionClone.mergeWith(existingRegion); + std::swap(existingRegion, regionClone); + } + + goto CONTINUE_OUTER; + } + + mergedRegions.emplace_back(region); + + CONTINUE_OUTER: + continue; + } + + std::sort( + mergedRegions.begin(), + mergedRegions.end(), + [] (const Region& regionA, const Region& regionB) { + return regionA.addressRange.startAddress < regionB.addressRange.startAddress; + } + ); + + auto output = std::vector{}; + auto deltaSegment = std::optional{}; + + /* + * Given that delta segments must be aligned to a given block size, not all bytes in a delta segment will + * differ from the corresponding bytes in the cached data. + * + * We enforce alignment of all delta segments from the point of creation, and maintain alignment during any + * subsequent changes. This simplifies things. + * + * We use the cached data to align the segments. Initially, we populate the aligned segments with the cached + * data, and then gradually overwrite the bytes that differ. + */ + for (const auto& region : mergedRegions) { + for (auto i = std::size_t{0}; i < region.buffer.size(); ++i) { + const auto address = static_cast(region.addressRange.startAddress + i); + const auto cacheIndex = static_cast( + address - this->memorySegmentDescriptor.addressRange.startAddress + ); + const auto regionByte = region.buffer[i]; + const auto cachedByte = cacheData[cacheIndex]; + + if (regionByte == cachedByte) { + continue; + } + + if (deltaSegment.has_value() && address > (deltaSegment->addressRange.endAddress + blockSize)) { + /* + * This region byte is not within the boundary of the current delta segment, or in the neighbouring + * block, so commit and create a new one. + */ + output.emplace_back(std::move(*deltaSegment)); + deltaSegment = std::nullopt; + } + + if (!deltaSegment.has_value()) { + const auto alignedAddress = AlignmentService::alignMemoryAddress(address, blockSize); + const auto alignedSize = AlignmentService::alignMemorySize((address - alignedAddress) + 1, blockSize); + + const auto cacheOffset = cacheData.begin() + static_cast( + alignedAddress - this->memorySegmentDescriptor.addressRange.startAddress + ); + deltaSegment = Region{ + .addressRange = TargetMemoryAddressRange{alignedAddress, alignedAddress + alignedSize - 1}, + .buffer = {cacheOffset, cacheOffset + alignedSize} + }; + } + + if (address > deltaSegment->addressRange.endAddress) { + /* + * This region byte is in the neighbouring block of the current delta segment. + * + * Instead of committing the segment and creating a new one, we just extend it by another block, + * to accommodate the byte. This reduces the number of segments (and therefore, the number of + * memory writes). + */ + const auto cacheOffset = cacheData.begin() + static_cast( + (deltaSegment->addressRange.endAddress + - this->memorySegmentDescriptor.addressRange.startAddress) + 1 + ); + deltaSegment->buffer.insert(deltaSegment->buffer.end(), cacheOffset, cacheOffset + blockSize); + deltaSegment->addressRange.endAddress += blockSize; + } + + deltaSegment->buffer[address - deltaSegment->addressRange.startAddress] = regionByte; + } + } + + if (deltaSegment.has_value()) { + output.emplace_back(std::move(*deltaSegment)); + } + + return output; + } + + void Session::pushEraseOperation( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor + ) { + if (this->eraseOperationsBySegmentId.contains(memorySegmentDescriptor.id)) { + return; + } + + this->eraseOperationsBySegmentId.emplace( + memorySegmentDescriptor.id, + EraseOperation{ + .addressSpaceDescriptor = addressSpaceDescriptor, + .memorySegmentDescriptor = memorySegmentDescriptor, + } + ); + } + + void Session::pushWriteOperation( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + TargetMemoryAddress startAddress, + Targets::TargetMemoryBuffer&& buffer + ) { + assert(!buffer.empty()); + + auto operationIt = this->writeOperationsBySegmentId.find(memorySegmentDescriptor.id); + if (operationIt == this->writeOperationsBySegmentId.end()) { + operationIt = this->writeOperationsBySegmentId.emplace( + memorySegmentDescriptor.id, + WriteOperation{ + .addressSpaceDescriptor = addressSpaceDescriptor, + .memorySegmentDescriptor = memorySegmentDescriptor, + .regions = {}, + } + ).first; + } + + operationIt->second.regions.emplace_back( + WriteOperation::Region{ + .addressRange = TargetMemoryAddressRange{ + startAddress, + static_cast(startAddress + buffer.size() - 1) + }, + .buffer = std::move(buffer) + } + ); + } +} diff --git a/src/Targets/DeltaProgramming/Session.hpp b/src/Targets/DeltaProgramming/Session.hpp new file mode 100644 index 00000000..bb8d25cc --- /dev/null +++ b/src/Targets/DeltaProgramming/Session.hpp @@ -0,0 +1,57 @@ +#pragma once + +#include +#include +#include + +#include "src/Targets/TargetAddressSpaceDescriptor.hpp" +#include "src/Targets/TargetMemorySegmentDescriptor.hpp" +#include "src/Targets/TargetMemory.hpp" +#include "src/Targets/TargetMemoryAddressRange.hpp" + +namespace Targets::DeltaProgramming +{ + struct Session + { + struct EraseOperation + { + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor; + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor; + }; + + struct WriteOperation + { + struct Region + { + Targets::TargetMemoryAddressRange addressRange; + Targets::TargetMemoryBuffer buffer; + + void mergeWith(const Region& other); + }; + + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor; + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor; + std::vector regions; + + [[nodiscard]] std::vector deltaSegments( + Targets::TargetMemoryBufferSpan cacheData, + Targets::TargetMemorySize blockSize + ) const; + }; + + std::unordered_map eraseOperationsBySegmentId; + std::unordered_map writeOperationsBySegmentId; + + void pushEraseOperation( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor + ); + + void pushWriteOperation( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + Targets::TargetMemoryAddress startAddress, + Targets::TargetMemoryBuffer&& buffer + ); + }; +} diff --git a/src/Targets/Microchip/Avr8/Avr8.cpp b/src/Targets/Microchip/Avr8/Avr8.cpp index a1cea6b4..14d69a3b 100644 --- a/src/Targets/Microchip/Avr8/Avr8.cpp +++ b/src/Targets/Microchip/Avr8/Avr8.cpp @@ -656,6 +656,7 @@ namespace Targets::Microchip::Avr8 return; } + this->avr8DebugInterface->clearAllBreakpoints(); this->avr8DebugInterface->enableProgrammingMode(); this->activeProgrammingSession = ProgrammingSession(); } @@ -687,6 +688,32 @@ namespace Targets::Microchip::Avr8 return std::nullopt; } + DeltaProgramming::DeltaProgrammingInterface* Avr8::deltaProgrammingInterface() { + if ( + this->targetConfig.physicalInterface == TargetPhysicalInterface::DEBUG_WIRE + || this->targetConfig.physicalInterface == TargetPhysicalInterface::UPDI + ) { + return this; + } + + return nullptr; + } + + TargetMemorySize Avr8::deltaBlockSize( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor + ) { + return memorySegmentDescriptor.pageSize.value_or(1); + } + + bool Avr8::shouldAbandonSession( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + const std::vector& deltaSegments + ) { + return false; + } + std::map Avr8::generateGpioPadDescriptorMapping( const std::vector& portPeripheralDescriptors ) { diff --git a/src/Targets/Microchip/Avr8/Avr8.hpp b/src/Targets/Microchip/Avr8/Avr8.hpp index 16d32af5..1ef8e272 100644 --- a/src/Targets/Microchip/Avr8/Avr8.hpp +++ b/src/Targets/Microchip/Avr8/Avr8.hpp @@ -22,6 +22,7 @@ #include "src/Targets/TargetBitFieldDescriptor.hpp" #include "src/Targets/TargetPadDescriptor.hpp" #include "src/Targets/TargetBreakpoint.hpp" +#include "src/Targets/DeltaProgramming/DeltaProgrammingInterface.hpp" #include "TargetDescriptionFile.hpp" @@ -29,7 +30,9 @@ namespace Targets::Microchip::Avr8 { - class Avr8: public Target + class Avr8 + : public Target + , public DeltaProgramming::DeltaProgrammingInterface { public: explicit Avr8(const TargetConfig& targetConfig, TargetDescriptionFile&& targetDescriptionFile); @@ -111,6 +114,17 @@ namespace Targets::Microchip::Avr8 std::string passthroughCommandHelpText() override; std::optional invokePassthroughCommand(const PassthroughCommand& command) override; + DeltaProgramming::DeltaProgrammingInterface* deltaProgrammingInterface() override; + TargetMemorySize deltaBlockSize( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor + ) override; + bool shouldAbandonSession( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + const std::vector& deltaSegments + ) override; + protected: DebugToolDrivers::TargetInterfaces::TargetPowerManagementInterface* targetPowerManagementInterface = nullptr; DebugToolDrivers::TargetInterfaces::Microchip::Avr8::Avr8DebugInterface* avr8DebugInterface = nullptr; diff --git a/src/Targets/ProgramBreakpointRegistry.hpp b/src/Targets/ProgramBreakpointRegistry.hpp index 214648a5..d9af1976 100644 --- a/src/Targets/ProgramBreakpointRegistry.hpp +++ b/src/Targets/ProgramBreakpointRegistry.hpp @@ -95,6 +95,14 @@ namespace Targets ); } + decltype(ProgramBreakpointRegistryGeneric::mapping)::iterator begin() noexcept { + return this->mapping.begin(); + } + + decltype(ProgramBreakpointRegistryGeneric::mapping)::iterator end() noexcept { + return this->mapping.end(); + } + decltype(ProgramBreakpointRegistryGeneric::mapping)::const_iterator begin() const noexcept { return this->mapping.begin(); } diff --git a/src/Targets/RiscV/ProgramBreakpoint.hpp b/src/Targets/RiscV/ProgramBreakpoint.hpp deleted file mode 100644 index 9ec246e9..00000000 --- a/src/Targets/RiscV/ProgramBreakpoint.hpp +++ /dev/null @@ -1,23 +0,0 @@ -#pragma once - -#include - -#include "src/Targets/TargetBreakpoint.hpp" -#include "Opcodes/Opcode.hpp" - -namespace Targets::RiscV -{ - struct ProgramBreakpoint: TargetProgramBreakpoint - { - std::optional originalInstruction = std::nullopt; - - explicit ProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) - : TargetProgramBreakpoint(breakpoint) - {} - - explicit ProgramBreakpoint(const TargetProgramBreakpoint& breakpoint, Opcodes::Opcode originalInstruction) - : TargetProgramBreakpoint(breakpoint) - , originalInstruction(originalInstruction) - {} - }; -} diff --git a/src/Targets/RiscV/Wch/WchRiscV.cpp b/src/Targets/RiscV/Wch/WchRiscV.cpp index cb4949a9..10332c38 100644 --- a/src/Targets/RiscV/Wch/WchRiscV.cpp +++ b/src/Targets/RiscV/Wch/WchRiscV.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -207,7 +208,8 @@ namespace Targets::RiscV::Wch .memorySegmentDescriptor = this->selectedProgramSegmentDescriptor, .address = this->deAliasMappedAddress(breakpoint.address, this->selectedProgramSegmentDescriptor), .size = breakpoint.size, - .type = breakpoint.type + .type = breakpoint.type, + .originalData = breakpoint.originalData }); return; @@ -233,7 +235,8 @@ namespace Targets::RiscV::Wch .memorySegmentDescriptor = this->selectedProgramSegmentDescriptor, .address = this->deAliasMappedAddress(breakpoint.address, this->selectedProgramSegmentDescriptor), .size = breakpoint.size, - .type = breakpoint.type + .type = breakpoint.type, + .originalData = breakpoint.originalData }); return; @@ -613,6 +616,48 @@ namespace Targets::RiscV::Wch return std::nullopt; } + DeltaProgramming::DeltaProgrammingInterface* WchRiscV::deltaProgrammingInterface() { + return this; + } + + TargetMemorySize WchRiscV::deltaBlockSize( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor + ) { + return 64; + } + + bool WchRiscV::shouldAbandonSession( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + const std::vector& deltaSegments + ) { + /* + * Delta programming isn't always faster on WCH RISC-V targets with WCH-Link debug tools. + * + * This is because the debug tool can write to program memory with two methods - one better suited for small + * operations and the other for larger ones. If there are many small delta segments, we'd end up using the + * slower method many times, resulting in a negative impact on programming speed. + * + * For this reason, we abandon delta sessions if they consist of too many delta segments. + * + * See WchLinkDebugInterface::writeMemory() for more. + * + * TODO: Consider moving this to the WCH-Link driver, seeing as it's specific to that debug tool. In fact, I + * think the entire implementation of the DeltaProgrammingInterface should reside in the tool driver, as + * opposed to the target driver. Something to look at after v2.0.0. + */ + return + deltaSegments.size() > 5 + || std::count_if( + deltaSegments.begin(), + deltaSegments.end(), + [] (const DeltaProgramming::Session::WriteOperation::Region& segment) { + return segment.buffer.size() > 192; + } + ) > 2; + } + const TargetMemorySegmentDescriptor& WchRiscV::resolveAliasedMemorySegment() { /* * To determine the aliased segment, we probe the boundary of the boot segment via the mapped segment. diff --git a/src/Targets/RiscV/Wch/WchRiscV.hpp b/src/Targets/RiscV/Wch/WchRiscV.hpp index 0180e43a..584f647d 100644 --- a/src/Targets/RiscV/Wch/WchRiscV.hpp +++ b/src/Targets/RiscV/Wch/WchRiscV.hpp @@ -9,6 +9,7 @@ #include "src/Targets/RiscV/RiscV.hpp" #include "src/Targets/TargetPeripheralDescriptor.hpp" #include "src/Targets/TargetPadDescriptor.hpp" +#include "src/Targets/DeltaProgramming/DeltaProgrammingInterface.hpp" #include "WchRiscVTargetConfig.hpp" #include "TargetDescriptionFile.hpp" @@ -21,7 +22,7 @@ namespace Targets::RiscV::Wch { class WchRiscV : public ::Targets::RiscV::RiscV - , public DeltaProgrammingInterface + , public DeltaProgramming::DeltaProgrammingInterface { public: WchRiscV(const TargetConfig& targetConfig, TargetDescriptionFile&& targetDescriptionFile); @@ -59,6 +60,17 @@ namespace Targets::RiscV::Wch std::string passthroughCommandHelpText() override; std::optional invokePassthroughCommand(const PassthroughCommand& command) override; + DeltaProgramming::DeltaProgrammingInterface* deltaProgrammingInterface() override; + TargetMemorySize deltaBlockSize( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor + ) override; + bool shouldAbandonSession( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + const std::vector& deltaSegments + ) override; + protected: WchRiscVTargetConfig targetConfig; TargetDescriptionFile targetDescriptionFile; diff --git a/src/Targets/Target.hpp b/src/Targets/Target.hpp index fe0b8f5e..0607538d 100644 --- a/src/Targets/Target.hpp +++ b/src/Targets/Target.hpp @@ -24,6 +24,8 @@ #include "PassthroughCommand.hpp" #include "PassthroughResponse.hpp" +#include "DeltaProgramming/DeltaProgrammingInterface.hpp" + #include "src/DebugToolDrivers/DebugTool.hpp" namespace Targets @@ -152,11 +154,19 @@ namespace Targets virtual TargetGpioPadDescriptorAndStatePairs getGpioPadStates(const TargetPadDescriptors& padDescriptors) = 0; virtual void setGpioPadState(const TargetPadDescriptor& padDescriptor, const TargetGpioPadState& state) = 0; + /** + * When enabling programming mode, the target driver is expected to clear all program breakpoints currently + * installed on the target. + * + * Before disabling program mode, the target controller will reinstall the necessary breakpoints. + */ virtual void enableProgrammingMode() = 0; virtual void disableProgrammingMode() = 0; virtual bool programmingModeEnabled() = 0; virtual std::string passthroughCommandHelpText() = 0; virtual std::optional invokePassthroughCommand(const PassthroughCommand& command) = 0; + + virtual DeltaProgramming::DeltaProgrammingInterface* deltaProgrammingInterface() = 0; }; } diff --git a/src/Targets/TargetBreakpoint.hpp b/src/Targets/TargetBreakpoint.hpp index 2aa3d57a..dbd334a9 100644 --- a/src/Targets/TargetBreakpoint.hpp +++ b/src/Targets/TargetBreakpoint.hpp @@ -1,8 +1,7 @@ #pragma once #include -#include -#include +#include #include "TargetMemory.hpp" #include "TargetAddressSpaceDescriptor.hpp" @@ -18,6 +17,8 @@ namespace Targets struct TargetProgramBreakpoint { + static constexpr auto MAX_SIZE = TargetMemorySize{4}; + enum class Type: std::uint8_t { HARDWARE, @@ -29,6 +30,7 @@ namespace Targets TargetMemoryAddress address; TargetMemorySize size; Type type; + std::array originalData; }; struct BreakpointResources diff --git a/src/Targets/TargetMemoryCache.cpp b/src/Targets/TargetMemoryCache.cpp index 7cc7c707..20d4c3d8 100644 --- a/src/Targets/TargetMemoryCache.cpp +++ b/src/Targets/TargetMemoryCache.cpp @@ -1,6 +1,7 @@ #include "TargetMemoryCache.hpp" #include +#include #include "src/Exceptions/Exception.hpp" @@ -11,7 +12,7 @@ namespace Targets , data(TargetMemoryBuffer(memorySegmentDescriptor.size(), 0x00)) {} - TargetMemoryBuffer TargetMemoryCache::fetch(TargetMemoryAddress startAddress, TargetMemorySize bytes) const { + TargetMemoryBufferSpan TargetMemoryCache::fetch(TargetMemoryAddress startAddress, TargetMemorySize bytes) const { const auto startIndex = startAddress - this->memorySegmentDescriptor.addressRange.startAddress; if ( @@ -21,7 +22,7 @@ namespace Targets throw Exceptions::Exception{"Invalid cache access"}; } - return TargetMemoryBuffer{this->data.begin() + startIndex, this->data.begin() + startIndex + bytes}; + return TargetMemoryBufferSpan{this->data.begin() + startIndex, this->data.begin() + startIndex + bytes}; } bool TargetMemoryCache::contains(TargetMemoryAddress startAddress, TargetMemorySize bytes) const { @@ -34,12 +35,30 @@ namespace Targets } void TargetMemoryCache::insert(TargetMemoryAddress startAddress, TargetMemoryBufferSpan data) { - const auto startIndex = startAddress - this->memorySegmentDescriptor.addressRange.startAddress; + std::copy( + data.begin(), + data.end(), + this->data.begin() + (startAddress - this->memorySegmentDescriptor.addressRange.startAddress) + ); - std::copy(data.begin(), data.end(), this->data.begin() + startIndex); + this->trackSegment(startAddress, static_cast(startAddress + data.size() - 1)); + } - const auto endAddress = static_cast(startAddress + data.size() - 1); + void TargetMemoryCache::fill(TargetMemoryAddress startAddress, TargetMemorySize size, unsigned char value) { + assert(this->data.size() >= (startAddress - this->memorySegmentDescriptor.addressRange.startAddress) + size); + for (auto i = std::size_t{0}; i < size; ++i) { + this->data[i + (startAddress - this->memorySegmentDescriptor.addressRange.startAddress)] = value; + } + + this->trackSegment(startAddress, static_cast(startAddress + size - 1)); + } + + void TargetMemoryCache::clear() { + this->populatedSegments.clear(); + } + + void TargetMemoryCache::trackSegment(TargetMemoryAddress startAddress, TargetMemoryAddress endAddress) { const auto intersectingStartSegmentIt = this->intersectingSegment(startAddress); const auto intersectingEndSegmentIt = this->intersectingSegment(endAddress); @@ -95,10 +114,6 @@ namespace Targets } } - void TargetMemoryCache::clear() { - this->populatedSegments.clear(); - } - TargetMemoryCache::SegmentIt TargetMemoryCache::intersectingSegment(TargetMemoryAddress address) const { if (this->populatedSegments.empty()) { return this->populatedSegments.end(); diff --git a/src/Targets/TargetMemoryCache.hpp b/src/Targets/TargetMemoryCache.hpp index 776a5f5c..3113ec26 100644 --- a/src/Targets/TargetMemoryCache.hpp +++ b/src/Targets/TargetMemoryCache.hpp @@ -11,47 +11,20 @@ namespace Targets class TargetMemoryCache { public: - explicit TargetMemoryCache(const TargetMemorySegmentDescriptor& memorySegmentDescriptor); - - /** - * Fetches data from the cache. - * - * TODO: Change return type to TargetMemoryBufferSpan - * - * @param startAddress - * @param bytes - * - * @return - */ - [[nodiscard]] TargetMemoryBuffer fetch(TargetMemoryAddress startAddress, TargetMemorySize bytes) const; - - /** - * Checks if the cache currently holds data within the given address range. - * - * @param startAddress - * @param bytes - * - * @return - */ - [[nodiscard]] bool contains(TargetMemoryAddress startAddress, TargetMemorySize bytes) const; - - /** - * Inserts data into the cache and performs any necessary bookkeeping. - * - * @param startAddress - * @param data - */ - void insert(TargetMemoryAddress startAddress, TargetMemoryBufferSpan data); - - /** - * Clears the cache. - */ - void clear(); - - private: const TargetMemorySegmentDescriptor& memorySegmentDescriptor; TargetMemoryBuffer data; + explicit TargetMemoryCache(const TargetMemorySegmentDescriptor& memorySegmentDescriptor); + + [[nodiscard]] TargetMemoryBufferSpan fetch(TargetMemoryAddress startAddress, TargetMemorySize bytes) const; + [[nodiscard]] bool contains(TargetMemoryAddress startAddress, TargetMemorySize bytes) const; + + void insert(TargetMemoryAddress startAddress, TargetMemoryBufferSpan data); + void fill(TargetMemoryAddress startAddress, TargetMemorySize size, unsigned char value); + + void clear(); + + private: /** * A populated segment is just an address range in the cache that we know we've populated. * @@ -60,6 +33,14 @@ namespace Targets */ std::map populatedSegments = {}; + /** + * Tracks a newly populated segment. + * + * @param startAddress + * @param endAddress + */ + void trackSegment(TargetMemoryAddress startAddress, TargetMemoryAddress endAddress); + /** * Finds the segment that intersects with the given address. Segments cannot overlap, so only one segment can * intersect with the given address, at any given time.