diff --git a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp index 2ed1210f..60b52605 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "src/Services/PathService.hpp" #include "src/Services/StringService.hpp" @@ -80,15 +81,18 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr using CommandFrames::Avr8Generic::DisableDebugWire; using Targets::TargetAddressSpaceDescriptor; + using Targets::TargetMemorySegmentDescriptor; using Targets::TargetMemorySegmentType; using Targets::TargetExecutionState; using Targets::TargetPhysicalInterface; using Targets::TargetMemoryBuffer; using Targets::TargetMemoryAddress; + using Targets::TargetMemoryAddressRange; using Targets::TargetMemorySize; using Targets::TargetRegisterDescriptor; using Targets::TargetRegisterDescriptors; using Targets::TargetRegisterDescriptorAndValuePairs; + using Targets::TargetProgramBreakpoint; EdbgAvr8Interface::EdbgAvr8Interface( EdbgInterface* edbgInterface, @@ -169,6 +173,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } this->cachedExecutionState = TargetExecutionState::RUNNING; + this->commitPendingBreakpointOperations(); } void EdbgAvr8Interface::runTo(TargetMemoryAddress address) { @@ -180,6 +185,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } this->cachedExecutionState = TargetExecutionState::RUNNING; + this->commitPendingBreakpointOperations(); } void EdbgAvr8Interface::step() { @@ -189,6 +195,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } this->cachedExecutionState = TargetExecutionState::STEPPING; + this->commitPendingBreakpointOperations(); } void EdbgAvr8Interface::reset() { @@ -360,80 +367,34 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr return responseFrame.extractSignature(this->session.targetConfig.physicalInterface); } - void EdbgAvr8Interface::setSoftwareBreakpoint(TargetMemoryAddress address) { - const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( - SetSoftwareBreakpoints{{address}} - ); + void EdbgAvr8Interface::setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { + this->setHardwareBreakpoint(breakpoint.address); - if (responseFrame.id == Avr8ResponseId::FAILED) { - throw Avr8CommandFailure{"AVR8 Set software breakpoint command failed", responseFrame}; - } - } - - void EdbgAvr8Interface::clearSoftwareBreakpoint(TargetMemoryAddress address) { - const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( - ClearSoftwareBreakpoints{{address}} - ); - - if (responseFrame.id == Avr8ResponseId::FAILED) { - throw Avr8CommandFailure{"AVR8 Clear software breakpoint command failed", responseFrame}; - } - } - - void EdbgAvr8Interface::setHardwareBreakpoint(TargetMemoryAddress address) { - static const auto getAvailableBreakpointNumbers = [this] () { - auto breakpointNumbers = std::set{1, 2, 3}; - - for (const auto& [address, allocatedNumber] : this->hardwareBreakpointNumbersByAddress) { - breakpointNumbers.erase(allocatedNumber); + } else { + if (this->pendingSoftwareBreakpointInsertions.contains(breakpoint)) { + return; } - return breakpointNumbers; - }; - - if (this->hardwareBreakpointNumbersByAddress.contains(address)) { - Logger::debug( - "Hardware breakpoint already installed for byte address 0x" + Services::StringService::toHex(address) - + " - ignoring request" - ); - return; + this->setSoftwareBreakpoint(breakpoint.address); + this->pendingSoftwareBreakpointInsertions.insert(breakpoint); + this->pendingSoftwareBreakpointDeletions.remove(breakpoint); } - - const auto availableBreakpointNumbers = getAvailableBreakpointNumbers(); - - if (availableBreakpointNumbers.empty()) { - throw Exception{"Maximum hardware breakpoints have been allocated"}; - } - - const auto breakpointNumber = *(availableBreakpointNumbers.begin()); - - const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( - SetHardwareBreakpoint{address, breakpointNumber} - ); - - if (responseFrame.id == Avr8ResponseId::FAILED) { - throw Avr8CommandFailure{"AVR8 Set hardware breakpoint command failed", responseFrame}; - } - - this->hardwareBreakpointNumbersByAddress.emplace(address, breakpointNumber); } - void EdbgAvr8Interface::clearHardwareBreakpoint(TargetMemoryAddress address) { - const auto breakpointNumberIt = this->hardwareBreakpointNumbersByAddress.find(address); - if (breakpointNumberIt == this->hardwareBreakpointNumbersByAddress.end()) { - Logger::error("No hardware breakpoint at byte address 0x" + Services::StringService::toHex(address)); - return; + void EdbgAvr8Interface::removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { + this->clearHardwareBreakpoint(breakpoint.address); + + } else { + if (this->pendingSoftwareBreakpointDeletions.contains(breakpoint)) { + return; + } + + this->clearSoftwareBreakpoint(breakpoint.address); + this->pendingSoftwareBreakpointDeletions.insert(breakpoint); + this->pendingSoftwareBreakpointInsertions.remove(breakpoint); } - - const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( - ClearHardwareBreakpoint{breakpointNumberIt->second} - ); - - if (responseFrame.id == Avr8ResponseId::FAILED) { - throw Avr8CommandFailure{"AVR8 Clear hardware breakpoint command failed", responseFrame}; - } - - this->hardwareBreakpointNumbersByAddress.erase(address); } TargetRegisterDescriptorAndValuePairs EdbgAvr8Interface::readRegisters( @@ -628,8 +589,8 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } TargetMemoryBuffer EdbgAvr8Interface::readMemory( - const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, - const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, TargetMemoryAddress startAddress, TargetMemorySize bytes, const std::set& excludedAddressRanges @@ -659,12 +620,30 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } if (memorySegmentDescriptor.type == TargetMemorySegmentType::FLASH) { + const auto injectAndConcealBreakpoints = [&] (TargetMemoryBuffer&& data) -> TargetMemoryBuffer&& { + this->injectActiveBreakpoints( + addressSpaceDescriptor, + memorySegmentDescriptor, + data, + startAddress + ); + this->concealPendingBreakpointOperations( + addressSpaceDescriptor, + memorySegmentDescriptor, + data, + startAddress + ); + return std::move(data); + }; + if (this->session.configVariant == Avr8ConfigVariant::MEGAJTAG) { - return this->readMemory( - this->programmingModeEnabled ? Avr8MemoryType::FLASH_PAGE : Avr8MemoryType::SPM, - startAddress, - bytes, - excludedAddresses + return injectAndConcealBreakpoints( + this->readMemory( + this->programmingModeEnabled ? Avr8MemoryType::FLASH_PAGE : Avr8MemoryType::SPM, + startAddress, + bytes, + excludedAddresses + ) ); } @@ -675,11 +654,13 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr * When using the BOOT_FLASH memory type, the address should be relative to the start of the * boot section. */ - return this->readMemory( - Avr8MemoryType::BOOT_FLASH, - startAddress - bootSectionStartAddress, - bytes, - excludedAddresses + return injectAndConcealBreakpoints( + this->readMemory( + Avr8MemoryType::BOOT_FLASH, + startAddress - bootSectionStartAddress, + bytes, + excludedAddresses + ) ); } @@ -687,15 +668,19 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr * When using the APPL_FLASH memory type, the address should be relative to the start of the * application section. */ - return this->readMemory( - Avr8MemoryType::APPL_FLASH, - startAddress - this->session.programAppSection.value().get().startAddress, - bytes, - excludedAddresses + return injectAndConcealBreakpoints( + this->readMemory( + Avr8MemoryType::APPL_FLASH, + startAddress - this->session.programAppSection.value().get().startAddress, + bytes, + excludedAddresses + ) ); } - return this->readMemory(Avr8MemoryType::FLASH_PAGE, startAddress, bytes, excludedAddresses); + return injectAndConcealBreakpoints( + this->readMemory(Avr8MemoryType::FLASH_PAGE, startAddress, bytes, excludedAddresses) + ); } if (memorySegmentDescriptor.type == TargetMemorySegmentType::EEPROM) { @@ -742,8 +727,8 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } void EdbgAvr8Interface::writeMemory( - const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, - const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, TargetMemoryAddress startAddress, Targets::TargetMemoryBufferSpan buffer ) { @@ -772,8 +757,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr buffer ); } - - return this->writeMemory(Avr8MemoryType::FLASH_PAGE, startAddress, buffer); } return this->writeMemory(Avr8MemoryType::FLASH_PAGE, startAddress, buffer); @@ -895,6 +878,9 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr * 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(); @@ -1232,6 +1218,82 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr this->targetAttached = false; } + void EdbgAvr8Interface::setSoftwareBreakpoint(TargetMemoryAddress address) { + const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( + SetSoftwareBreakpoints{{address}} + ); + + if (responseFrame.id == Avr8ResponseId::FAILED) { + throw Avr8CommandFailure{"AVR8 Set software breakpoint command failed", responseFrame}; + } + } + + void EdbgAvr8Interface::clearSoftwareBreakpoint(TargetMemoryAddress address) { + const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( + ClearSoftwareBreakpoints{{address}} + ); + + if (responseFrame.id == Avr8ResponseId::FAILED) { + throw Avr8CommandFailure{"AVR8 Clear software breakpoint command failed", responseFrame}; + } + } + + void EdbgAvr8Interface::setHardwareBreakpoint(TargetMemoryAddress address) { + static const auto getAvailableBreakpointNumbers = [this] () { + auto breakpointNumbers = std::set{1, 2, 3}; + + for (const auto& [address, allocatedNumber] : this->hardwareBreakpointNumbersByAddress) { + breakpointNumbers.erase(allocatedNumber); + } + + return breakpointNumbers; + }; + + if (this->hardwareBreakpointNumbersByAddress.contains(address)) { + Logger::debug( + "Hardware breakpoint already installed for byte address 0x" + Services::StringService::toHex(address) + + " - ignoring request" + ); + return; + } + + const auto availableBreakpointNumbers = getAvailableBreakpointNumbers(); + + if (availableBreakpointNumbers.empty()) { + throw Exception{"Maximum hardware breakpoints have been allocated"}; + } + + const auto breakpointNumber = *(availableBreakpointNumbers.begin()); + + const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( + SetHardwareBreakpoint{address, breakpointNumber} + ); + + if (responseFrame.id == Avr8ResponseId::FAILED) { + throw Avr8CommandFailure{"AVR8 Set hardware breakpoint command failed", responseFrame}; + } + + this->hardwareBreakpointNumbersByAddress.emplace(address, breakpointNumber); + } + + void EdbgAvr8Interface::clearHardwareBreakpoint(TargetMemoryAddress address) { + const auto breakpointNumberIt = this->hardwareBreakpointNumbersByAddress.find(address); + if (breakpointNumberIt == this->hardwareBreakpointNumbersByAddress.end()) { + Logger::error("No hardware breakpoint at byte address 0x" + Services::StringService::toHex(address)); + return; + } + + const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( + ClearHardwareBreakpoint{breakpointNumberIt->second} + ); + + if (responseFrame.id == Avr8ResponseId::FAILED) { + throw Avr8CommandFailure{"AVR8 Clear hardware breakpoint command failed", responseFrame}; + } + + this->hardwareBreakpointNumbersByAddress.erase(address); + } + void EdbgAvr8Interface::clearAllSoftwareBreakpoints() { const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( ClearAllSoftwareBreakpoints{} @@ -1240,6 +1302,10 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr if (responseFrame.id == Avr8ResponseId::FAILED) { throw Avr8CommandFailure{"AVR8 Clear all software breakpoints command failed", responseFrame}; } + + this->activeSoftwareBreakpoints.clear(); + this->pendingSoftwareBreakpointInsertions.clear(); + this->pendingSoftwareBreakpointDeletions.clear(); } void EdbgAvr8Interface::clearAllBreakpoints() { @@ -1251,6 +1317,164 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } } + void EdbgAvr8Interface::injectActiveBreakpoints( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + TargetMemoryBuffer& data, + TargetMemoryAddress startAddress + ) { + if (data.empty()) { + return; + } + + for (const auto& [addressSpaceId, breakpointsByAddress] : this->activeSoftwareBreakpoints) { + if (addressSpaceId != addressSpaceDescriptor.id) { + continue; + } + + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->injectBreakOpcodeAtBreakpoint( + breakpoint, + addressSpaceDescriptor, + memorySegmentDescriptor, + data, + startAddress + ); + } + } + } + + void EdbgAvr8Interface::concealPendingBreakpointOperations( + const TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const TargetMemorySegmentDescriptor& memorySegmentDescriptor, + TargetMemoryBuffer& data, + TargetMemoryAddress startAddress + ) { + if (data.empty()) { + return; + } + + for (const auto& [addressSpaceId, breakpointsByAddress] : this->pendingSoftwareBreakpointInsertions) { + if (addressSpaceId != addressSpaceDescriptor.id) { + continue; + } + + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->injectBreakOpcodeAtBreakpoint( + breakpoint, + addressSpaceDescriptor, + memorySegmentDescriptor, + data, + startAddress + ); + } + } + + for (const auto& [addressSpaceId, breakpointsByAddress] : this->pendingSoftwareBreakpointDeletions) { + if (addressSpaceId != addressSpaceDescriptor.id) { + continue; + } + + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->injectOriginalOpcodeAtBreakpoint( + breakpoint, + addressSpaceDescriptor, + memorySegmentDescriptor, + data, + startAddress + ); + } + } + } + + void EdbgAvr8Interface::commitPendingBreakpointOperations() { + for (const auto& [addressSpaceId, breakpointsByAddress] : this->pendingSoftwareBreakpointInsertions) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->activeSoftwareBreakpoints.insert(breakpoint); + } + } + + for (const auto& [addressSpaceId, breakpointsByAddress] : this->pendingSoftwareBreakpointDeletions) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->activeSoftwareBreakpoints.remove(breakpoint); + } + } + + this->pendingSoftwareBreakpointInsertions.clear(); + this->pendingSoftwareBreakpointDeletions.clear(); + } + + void EdbgAvr8Interface::injectBreakOpcodeAtBreakpoint( + const Targets::TargetProgramBreakpoint& breakpoint, + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + TargetMemoryBuffer& data, + Targets::TargetMemoryAddress startAddress + ) { + static constexpr auto breakOpcode = std::to_array({0x98, 0x95}); + assert(breakpoint.size == breakOpcode.size()); + + const auto addressRange = TargetMemoryAddressRange{ + startAddress, + static_cast(startAddress + (data.size() / addressSpaceDescriptor.unitSize) - 1) + }; + + const auto breakpointRange = TargetMemoryAddressRange{ + breakpoint.address, + breakpoint.address + breakpoint.size - 1 + }; + + const auto intersectingSize = addressRange.intersectingSize(breakpointRange); + if (intersectingSize == 0) { + return; + } + + const auto addressOffset = breakpointRange.startAddress < addressRange.startAddress + ? addressRange.startAddress - breakpointRange.startAddress + : 0; + const auto breakOpcodeOffset = breakOpcode.begin() + addressOffset; + + std::copy( + breakOpcodeOffset, + breakOpcodeOffset + intersectingSize, + data.begin() + (breakpointRange.startAddress - addressRange.startAddress + addressOffset) + ); + } + + void EdbgAvr8Interface::injectOriginalOpcodeAtBreakpoint( + const Targets::TargetProgramBreakpoint& breakpoint, + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + TargetMemoryBuffer& data, + Targets::TargetMemoryAddress startAddress + ) { + const auto addressRange = TargetMemoryAddressRange{ + startAddress, + static_cast(startAddress + (data.size() / addressSpaceDescriptor.unitSize) - 1) + }; + + const auto breakpointRange = TargetMemoryAddressRange{ + breakpoint.address, + breakpoint.address + breakpoint.size - 1 + }; + + const auto intersectingSize = addressRange.intersectingSize(breakpointRange); + if (intersectingSize == 0) { + return; + } + + const auto addressOffset = breakpointRange.startAddress < addressRange.startAddress + ? addressRange.startAddress - breakpointRange.startAddress + : 0; + const auto originalDataOffset = breakpoint.originalData.begin() + addressOffset; + + std::copy( + originalDataOffset, + originalDataOffset + intersectingSize, + data.begin() + (breakpointRange.startAddress - addressRange.startAddress + addressOffset) + ); + } + std::unique_ptr EdbgAvr8Interface::getAvrEvent() { auto event = this->edbgInterface->requestAvrEvent(); @@ -1510,7 +1734,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } auto data = responseFrame.getMemoryData(); - if (data.size() != bytes) { throw Avr8CommandFailure{"Unexpected number of bytes returned from EDBG debug tool"}; } diff --git a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp index 02ae9148..f96af426 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.hpp @@ -14,6 +14,7 @@ #include "src/Targets/TargetPhysicalInterface.hpp" #include "src/Targets/TargetMemory.hpp" +#include "src/Targets/ProgramBreakpointRegistry.hpp" #include "src/Targets/TargetRegisterDescriptor.hpp" #include "src/Targets/Microchip/Avr8/TargetDescriptionFile.hpp" #include "src/Targets/Microchip/Avr8/Family.hpp" @@ -65,8 +66,8 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr * memory command that exceeds 256 bytes in the size of the read request, despite the fact that the HID report * size is 512 bytes. The debug tool doesn't report any error, it just returns incorrect data. * - * To address this, debug tool drivers can set a soft limit on the number of bytes this EdbgAvr8Interface instance - * will attempt to access in a single request, using the EdbgAvr8Interface::setMaximumMemoryAccessSizePerRequest() + * To address this, debug tool drivers can set a soft limit on the number of bytes this EdbgAvr8Interface + * instance will attempt to access in a single request, using the EdbgAvr8Interface::setMaximumMemoryAccessSizePerRequest() * member function. * * This limit will be enforced in all forms of memory access on the AVR8 target, including register access. @@ -91,51 +92,15 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr * See the comments in that class for more info on the expected behaviour of each method. */ - /** - * Initialises the AVR8 Generic protocol interface by setting the appropriate parameters on the debug tool. - */ void init() override; - /** - * Issues the "stop" command to the debug tool, halting target execution. - */ void stop() override; - - /** - * Issues the "run" command to the debug tool, resuming execution on the target. - */ void run() override; - - /** - * Issues the "run to" command to the debug tool, resuming execution on the target, up to a specific byte - * address. The target will dispatch an AVR BREAK event once it reaches the specified address. - * - * @param address - * The (byte) address to run to. - */ void runTo(Targets::TargetMemoryAddress address) override; - - /** - * Issues the "step" command to the debug tool, stepping the execution on the target. The stepping can be - * configured to step in, out or over. But currently we only support stepping in. The target will dispatch - * an AVR BREAK event once it reaches the next instruction. - */ void step() override; - - /** - * Issues the "reset" command to the debug tool, resetting target execution. - */ void reset() override; - /** - * Activates the physical interface and starts a debug session on the target (via attach()). - */ void activate() override; - - /** - * Terminates any active debug session on the target and severs the connection between the debug tool and - * the target (by deactivating the physical interface). - */ void deactivate() override; void applyAccessRestrictions( @@ -143,92 +108,19 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor ) override; - /** - * Issues the "PC Read" command to the debug tool, to extract the current program counter. - * - * @return - */ Targets::TargetMemoryAddress getProgramCounter() override; - - /** - * Issues the "PC Write" command to the debug tool, setting the program counter on the target. - * - * @param programCounter - * The byte address to set as the program counter. - */ void setProgramCounter(Targets::TargetMemoryAddress programCounter) override; - /** - * Issues the "Get ID" command to the debug tool, to extract the signature from the target. - * - * @return - */ Targets::Microchip::Avr8::TargetSignature getDeviceId() override; - /** - * Issues the "Software Breakpoint Set" command to the debug tool, setting a software breakpoint at the given - * byte address. - * - * @param address - * The byte address to place the breakpoint. - */ - void setSoftwareBreakpoint(Targets::TargetMemoryAddress address) override; + virtual void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; + virtual void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; - /** - * Issues the "Software Breakpoint Clear" command to the debug tool, clearing any software breakpoint at the - * given byte address. - * - * @param address - * The byte address of the breakpoint to clear. - */ - void clearSoftwareBreakpoint(Targets::TargetMemoryAddress address) override; - - /** - * Issues the "Hardware Breakpoint Set" command to the debug tool, setting a hardware breakpoint at the given - * byte address. - * - * @param address - * The byte address to place the breakpoint. - */ - void setHardwareBreakpoint(Targets::TargetMemoryAddress address) override; - - /** - * Issues the "Hardware Breakpoint Clear" command to the debug tool, clearing any hardware breakpoint at the - * given byte address. - * - * @param address - * The byte address of the breakpoint to clear. - */ - void clearHardwareBreakpoint(Targets::TargetMemoryAddress address) override; - - /** - * Reads registers from the target. - * - * @param descriptorIds - * @return - */ Targets::TargetRegisterDescriptorAndValuePairs readRegisters( const Targets::TargetRegisterDescriptors& descriptors ) override; - - /** - * Writes registers to target. - * - * @param registers - */ void writeRegisters(const Targets::TargetRegisterDescriptorAndValuePairs& registers) override; - /** - * This is an overloaded method. - * - * Resolves the correct Avr8MemoryType from the given TargetMemoryType and calls readMemory(). - * - * @param addressSpaceDescriptor - * @param memorySegmentDescriptor - * @param startAddress - * @param bytes - * @return - */ Targets::TargetMemoryBuffer readMemory( const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, @@ -236,17 +128,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr Targets::TargetMemorySize bytes, const std::set& excludedAddressRanges = {} ) override; - - /** - * This is an overloaded method. - * - * Resolves the correct Avr8MemoryType from the given TargetMemoryType and calls writeMemory(). - * - * @param addressSpaceDescriptor - * @param memorySegmentDescriptor - * @param startAddress - * @param buffer - */ void writeMemory( const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, @@ -254,86 +135,57 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr Targets::TargetMemoryBufferSpan buffer ) override; - /** - * Issues the "Erase" command to erase a particular section of program memory. - * - * @param section - */ void eraseProgramMemory( std::optional section = std::nullopt ) override; - - /** - * Issues the "Erase" command to erase the entire chip. - */ void eraseChip() override; - /** - * Returns the current state of the target. - * - * @return - */ Targets::TargetExecutionState getExecutionState() override; - /** - * Enters programming mode on the EDBG debug tool. - */ void enableProgrammingMode() override; - - /** - * Leaves programming mode on the EDBG debug tool. - */ void disableProgrammingMode() override; private: - /** - * The AVR8 Generic protocol is a sub-protocol of the EDBG AVR protocol, which is served via CMSIS-DAP vendor - * commands. - * - * Every EDBG based debug tool that utilises this implementation must provide access to its EDBG interface. - */ EdbgInterface* edbgInterface = nullptr; - - /** - * The active EDBG AVR8 session. - */ EdbgAvr8Session session; - /** - * See the comment for EdbgAvr8Interface::setAvoidMaskedMemoryRead(). - */ bool avoidMaskedMemoryRead = true; - - /** - * See the comment for EdbgAvr8Interface::setMaximumMemoryAccessSizePerRequest(). - */ std::optional maximumMemoryAccessSizePerRequest; - bool reactivateJtagTargetPostProgrammingMode = false; - - /** - * We keep record of the current target execution state for caching purposes. - * - * We'll only refresh the target state if the target is running. If it has already stopped, then we assume it - * cannot transition to a running state without an instruction from us. - * - * @TODO: Review this. Is the above assumption correct? Always? Explore the option of polling the target state. - */ Targets::TargetExecutionState cachedExecutionState = Targets::TargetExecutionState::UNKNOWN; - - /** - * Upon configuration, the physical interface must be activated on the debug tool. We keep record of this to - * assist in our decision to deactivate the physical interface, when deactivate() is called. - */ bool physicalInterfaceActivated = false; + bool targetAttached = false; + bool programmingModeEnabled = false; /** - * As with activating the physical interface, we are required to issue the "attach" command to the debug - * tool, in order to start a debug session in the target. + * The TargetController refreshes the relevant program memory cache after inserting/removing software + * breakpoints. So it expects the insertion/removal operation to take place immediately. But, EDBG tools do + * not appear to support this. They queue the insertion/removal of software breakpoints until the next + * program flow control command. + * + * To get around this, we keep track of the pending operations and intercept program memory access operations + * to inject the relevant instruction opcodes, so that it appears as if the breakpoints have been + * inserted/removed. In other words: we fake it - we make it seem as if the breakpoints were inserted/removed + * immediately, when in actuality, the operation is still pending. + * + * It's horrible, but I can't think of a better way. Yes, I have considered loosening the restriction on the + * target driver, but I dislike that approach even more, because it would mean making the rest of Bloom + * accommodate the "I'll do it when I feel like it" nature of the EDBG tool, which is something I really + * don't want to do. + * + * This also addresses another potential issue, where EDBG tools will filter out software breakpoints when + * accessing program memory via some memory types. This is undesired behaviour. We negate it by injecting + * BREAK opcodes at the addresses of all active software breakpoints, when accessing program memory via any + * memory type. + * + * For more, see the following member functions: + * - EdbgAvr8Interface::injectActiveBreakpoints() + * - EdbgAvr8Interface::concealPendingBreakpointOperations() + * - EdbgAvr8Interface::commitPendingBreakpointOperations() */ - bool targetAttached = false; - - bool programmingModeEnabled = false; + Targets::ProgramBreakpointRegistry pendingSoftwareBreakpointInsertions; + Targets::ProgramBreakpointRegistry pendingSoftwareBreakpointDeletions; + Targets::ProgramBreakpointRegistry activeSoftwareBreakpoints; /** * Every hardware breakpoint is assigned a "breakpoint number", which we need to keep track of in order to @@ -341,38 +193,11 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr */ std::map hardwareBreakpointNumbersByAddress; - /** - * Sends the necessary target parameters to the debug tool. - * - * @param config - */ void setTargetParameters(); - - /** - * Sets an AVR8 parameter on the debug tool. See the Avr8EdbgParameters class and protocol documentation - * for more on available parameters. - * - * @param parameter - * @param value - */ void setParameter(const Avr8EdbgParameter& parameter, const std::vector& value); - - /** - * Overload for setting parameters with single byte values. - * - * @param parameter - * @param value - */ void setParameter(const Avr8EdbgParameter& parameter, std::uint8_t value) { this->setParameter(parameter, std::vector(1, value)); } - - /** - * Overload for setting parameters with four byte integer values. - * - * @param parameter - * @param value - */ void setParameter(const Avr8EdbgParameter& parameter, std::uint32_t value) { auto paramValue = std::vector(4); paramValue[0] = static_cast(value); @@ -382,13 +207,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr this->setParameter(parameter, paramValue); } - - /** - * Overload for setting parameters with two byte integer values. - * - * @param parameter - * @param value - */ void setParameter(const Avr8EdbgParameter& parameter, std::uint16_t value) { auto paramValue = std::vector(2); paramValue[0] = static_cast(value); @@ -397,13 +215,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr this->setParameter(parameter, paramValue); } - /** - * Fetches an AV8 parameter from the debug tool. - * - * @param parameter - * @param size - * @return - */ std::vector getParameter(const Avr8EdbgParameter& parameter, std::uint8_t size); /** @@ -425,91 +236,55 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr void setPdiParameters(); void setUpdiParameters(); - /** - * Sends the "Activate Physical" command to the debug tool, activating the physical interface and thus enabling - * communication between the debug tool and the target. - * - * @param applyExternalReset - */ void activatePhysical(bool applyExternalReset = false); - - /** - * Sends the "Deactivate Physical" command to the debug tool, which will result in the connection between the - * debug tool and the target being severed. - */ void deactivatePhysical(); - /** - * Sends the "Attach" command to the debug tool, which starts a debug session on the target. - */ void attach(); - - /** - * Sends the "Detach" command to the debug tool, which terminates any active debug session on the target. - */ void detach(); - /** - * Issues the "Software Breakpoint Clear All" command to the debug tool, clearing all software breakpoints - * immediately. - */ + void setSoftwareBreakpoint(Targets::TargetMemoryAddress address); + void clearSoftwareBreakpoint(Targets::TargetMemoryAddress address); + void setHardwareBreakpoint(Targets::TargetMemoryAddress address); + void clearHardwareBreakpoint(Targets::TargetMemoryAddress address); void clearAllSoftwareBreakpoints(); - - /** - * Clears all software and hardware breakpoints on the target. - * - * This function will not clear any untracked hardware breakpoints (breakpoints that were installed in a - * previous session). - */ void clearAllBreakpoints(); + void injectActiveBreakpoints( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + Targets::TargetMemoryBuffer& data, + Targets::TargetMemoryAddress startAddress + ); + void concealPendingBreakpointOperations( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + Targets::TargetMemoryBuffer& data, + Targets::TargetMemoryAddress startAddress + ); + void commitPendingBreakpointOperations(); + void injectBreakOpcodeAtBreakpoint( + const Targets::TargetProgramBreakpoint& breakpoint, + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + Targets::TargetMemoryBuffer& data, + Targets::TargetMemoryAddress startAddress + ); + void injectOriginalOpcodeAtBreakpoint( + const Targets::TargetProgramBreakpoint& breakpoint, + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + Targets::TargetMemoryBuffer& data, + Targets::TargetMemoryAddress startAddress + ); - /** - * Fetches any queued events belonging to the AVR8 Generic protocol (such as target break events). - * - * @return - */ std::unique_ptr getAvrEvent(); - - /** - * Clears (discards) any queued AVR8 Generic protocol events on the debug tool. - */ void clearEvents(); Avr8MemoryType getRegisterMemoryType(const Targets::TargetRegisterDescriptor& descriptor); - /** - * Checks if alignment is required for memory access via a given Avr8MemoryType. - * - * @param memoryType - * @return - */ bool alignmentRequired(Avr8MemoryType memoryType); - - /** - * Aligns a memory address for a given memory type's page size. - * - * @param memoryType - * @param address - * @return - */ Targets::TargetMemoryAddress alignMemoryAddress(Avr8MemoryType memoryType, Targets::TargetMemoryAddress address); - - /** - * Aligns a number of bytes used to address memory, for a given memory type's page size. - * - * @param memoryType - * @param bytes - * @return - */ Targets::TargetMemorySize alignMemoryBytes(Avr8MemoryType memoryType, Targets::TargetMemorySize bytes); - /** - * Determines the maximum memory access size imposed on the given Avr8MemoryType. - * - * @param memoryType - * - * @return - */ Targets::TargetMemorySize maximumMemoryAccessSize(Avr8MemoryType memoryType); /** @@ -558,18 +333,8 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr Targets::TargetMemoryBufferSpan buffer ); - /** - * Fetches the current target state. - * - * This currently uses AVR BREAK events to determine if a target has stopped. The lack of any - * queued BREAK events leads to the assumption that the target is still running. - */ void refreshTargetState(); - /** - * Temporarily disables the debugWIRE module on the target. This does not affect the DWEN fuse. The module - * will be reactivated upon the cycling of the target power. - */ void disableDebugWire(); /** @@ -586,7 +351,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr * will be returned. */ template - std::unique_ptr waitForAvrEvent(int maximumAttempts = 20) { + std::unique_ptr waitForAvrEvent(int maximumAttempts = 60) { int attemptCount = 0; while (attemptCount <= maximumAttempts) { @@ -601,21 +366,12 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } } - std::this_thread::sleep_for(std::chrono::milliseconds{50}); + std::this_thread::sleep_for(std::chrono::milliseconds{5}); attemptCount++; } return nullptr; } - - /** - * Waits for an AVR BREAK event. - * - * This simply wraps a call to waitForAvrEvent(). An exception will be thrown if the call doesn't - * return a BreakEvent. - * - * This should only be used when a BreakEvent is always expected. - */ void waitForStoppedEvent(); }; } diff --git a/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp index a48ed0b7..bd92fa21 100644 --- a/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/Microchip/Avr8/Avr8DebugInterface.hpp @@ -15,6 +15,7 @@ #include "src/Targets/TargetState.hpp" #include "src/Targets/TargetRegisterDescriptor.hpp" #include "src/Targets/TargetMemory.hpp" +#include "src/Targets/TargetBreakpoint.hpp" namespace DebugToolDrivers::TargetInterfaces::Microchip::Avr8 { @@ -95,10 +96,8 @@ namespace DebugToolDrivers::TargetInterfaces::Microchip::Avr8 */ virtual Targets::Microchip::Avr8::TargetSignature getDeviceId() = 0; - virtual void setSoftwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - virtual void clearSoftwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - virtual void setHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - virtual void clearHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; + virtual void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) = 0; + virtual void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) = 0; virtual Targets::TargetMemoryAddress getProgramCounter() = 0; virtual void setProgramCounter(Targets::TargetMemoryAddress programCounter) = 0; diff --git a/src/Targets/Microchip/Avr8/Avr8.cpp b/src/Targets/Microchip/Avr8/Avr8.cpp index 55704797..a1cea6b4 100644 --- a/src/Targets/Microchip/Avr8/Avr8.cpp +++ b/src/Targets/Microchip/Avr8/Avr8.cpp @@ -367,28 +367,18 @@ namespace Targets::Microchip::Avr8 void Avr8::setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { if (breakpoint.addressSpaceDescriptor != this->programAddressSpaceDescriptor) { - throw Exception{"Unexpected address space descriptor"}; + throw Exception{"Unexpected address space"}; } - if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { - this->avr8DebugInterface->setHardwareBreakpoint(breakpoint.address); - - } else { - this->avr8DebugInterface->setSoftwareBreakpoint(breakpoint.address); - } + this->avr8DebugInterface->setProgramBreakpoint(breakpoint); } void Avr8::removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { if (breakpoint.addressSpaceDescriptor != this->programAddressSpaceDescriptor) { - throw Exception{"Unexpected address space descriptor"}; + throw Exception{"Unexpected address space"}; } - if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { - this->avr8DebugInterface->clearHardwareBreakpoint(breakpoint.address); - - } else { - this->avr8DebugInterface->clearSoftwareBreakpoint(breakpoint.address); - } + this->avr8DebugInterface->removeProgramBreakpoint(breakpoint); } TargetRegisterDescriptorAndValuePairs Avr8::readRegisters(const Targets::TargetRegisterDescriptors& descriptors) {