diff --git a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp index 840bccea..c8513f0e 100644 --- a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp +++ b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp @@ -159,7 +159,7 @@ namespace DebugServer::Gdb::AvrGdb * * We need to figure out why, and determine whether the stop should be reported to GDB. */ - if (this->activeDebugSession->externalBreakpointAddresses.contains(programAddress)) { + if (this->activeDebugSession->externalBreakpointsByAddress.contains(programAddress)) { /* * The target stopped due to an external breakpoint, set by GDB. * diff --git a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp b/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp index 79555199..c8bf8dcc 100644 --- a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp +++ b/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp @@ -56,7 +56,7 @@ namespace DebugServer::Gdb::CommandPackets try { Logger::debug("Removing breakpoint at address " + std::to_string(this->address)); - debugSession.removeExternalBreakpoint(TargetBreakpoint(this->address), targetControllerService); + debugSession.removeExternalBreakpoint(this->address, targetControllerService); debugSession.connection.writePacket(OkResponsePacket()); } catch (const Exception& exception) { diff --git a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp b/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp index 285f9abd..b0adff75 100644 --- a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp +++ b/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp @@ -65,7 +65,7 @@ namespace DebugServer::Gdb::CommandPackets return; } - debugSession.setExternalBreakpoint(TargetBreakpoint(this->address), targetControllerService); + debugSession.setExternalBreakpoint(this->address, targetControllerService); debugSession.connection.writePacket(OkResponsePacket()); } catch (const Exception& exception) { diff --git a/src/DebugServer/Gdb/DebugSession.cpp b/src/DebugServer/Gdb/DebugSession.cpp index b321d766..cd0b3e0b 100644 --- a/src/DebugServer/Gdb/DebugSession.cpp +++ b/src/DebugServer/Gdb/DebugSession.cpp @@ -27,63 +27,83 @@ namespace DebugServer::Gdb } void DebugSession::setInternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ) { - if (this->internalBreakpointAddresses.contains(breakpoint.address)) { + if (this->internalBreakpointsByAddress.contains(address)) { return; } - if (!this->externalBreakpointAddresses.contains(breakpoint.address)) { - targetControllerService.setBreakpoint(breakpoint); + const auto externalBreakpointIt = this->externalBreakpointsByAddress.find(address); + + if (externalBreakpointIt != this->externalBreakpointsByAddress.end()) { + // We already have an external breakpoint at this address + this->internalBreakpointsByAddress.insert(std::pair(address, externalBreakpointIt->second)); + return; } - this->internalBreakpointAddresses.insert(breakpoint.address); + this->internalBreakpointsByAddress.insert( + std::pair( + address, + targetControllerService.setBreakpoint(address, Targets::TargetBreakpoint::Type::HARDWARE) + ) + ); } void DebugSession::removeInternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ) { - if (!this->internalBreakpointAddresses.contains(breakpoint.address)) { + const auto breakpointIt = this->internalBreakpointsByAddress.find(address); + if (breakpointIt == this->internalBreakpointsByAddress.end()) { return; } - if (!this->externalBreakpointAddresses.contains(breakpoint.address)) { - targetControllerService.removeBreakpoint(breakpoint); + if (!this->externalBreakpointsByAddress.contains(address)) { + targetControllerService.removeBreakpoint(breakpointIt->second); } - this->internalBreakpointAddresses.erase(breakpoint.address); + this->internalBreakpointsByAddress.erase(breakpointIt); } void DebugSession::setExternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ) { - if (this->externalBreakpointAddresses.contains(breakpoint.address)) { + if (this->externalBreakpointsByAddress.contains(address)) { return; } - if (!this->internalBreakpointAddresses.contains(breakpoint.address)) { - targetControllerService.setBreakpoint(breakpoint); + const auto internalBreakpointIt = this->internalBreakpointsByAddress.find(address); + + if (internalBreakpointIt != this->internalBreakpointsByAddress.end()) { + // We already have an internal breakpoint at this address + this->externalBreakpointsByAddress.insert(std::pair(address, internalBreakpointIt->second)); + return; } - this->externalBreakpointAddresses.insert(breakpoint.address); + this->externalBreakpointsByAddress.insert( + std::pair( + address, + targetControllerService.setBreakpoint(address, Targets::TargetBreakpoint::Type::HARDWARE) + ) + ); } void DebugSession::removeExternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ) { - if (!this->externalBreakpointAddresses.contains(breakpoint.address)) { + const auto breakpointIt = this->externalBreakpointsByAddress.find(address); + if (breakpointIt == this->externalBreakpointsByAddress.end()) { return; } - if (!this->internalBreakpointAddresses.contains(breakpoint.address)) { - targetControllerService.removeBreakpoint(breakpoint); + if (!this->internalBreakpointsByAddress.contains(address)) { + targetControllerService.removeBreakpoint(breakpointIt->second); } - this->externalBreakpointAddresses.erase(breakpoint.address); + this->externalBreakpointsByAddress.erase(breakpointIt); } void DebugSession::startRangeSteppingSession( @@ -91,7 +111,7 @@ namespace DebugServer::Gdb Services::TargetControllerService& targetControllerService ) { for (const auto& interceptAddress : session.interceptedAddresses) { - this->setInternalBreakpoint(Targets::TargetBreakpoint(interceptAddress), targetControllerService); + this->setInternalBreakpoint(interceptAddress, targetControllerService); } this->activeRangeSteppingSession = std::move(session); @@ -104,7 +124,7 @@ namespace DebugServer::Gdb // Clear all intercepting breakpoints for (const auto& interceptAddress : this->activeRangeSteppingSession->interceptedAddresses) { - this->removeInternalBreakpoint(Targets::TargetBreakpoint(interceptAddress), targetControllerService); + this->removeInternalBreakpoint(interceptAddress, targetControllerService); } this->activeRangeSteppingSession.reset(); diff --git a/src/DebugServer/Gdb/DebugSession.hpp b/src/DebugServer/Gdb/DebugSession.hpp index 9d0eb28c..dcbf5231 100644 --- a/src/DebugServer/Gdb/DebugSession.hpp +++ b/src/DebugServer/Gdb/DebugSession.hpp @@ -2,7 +2,7 @@ #include #include -#include +#include #include "TargetDescriptor.hpp" #include "GdbDebugServerConfig.hpp" @@ -55,8 +55,8 @@ namespace DebugServer::Gdb * * We track internal and external breakpoints separately. */ - std::set internalBreakpointAddresses; - std::set externalBreakpointAddresses; + std::map internalBreakpointsByAddress; + std::map externalBreakpointsByAddress; /** * When the GDB client is waiting for the target to halt, this is set to true so we know when to notify the @@ -105,22 +105,22 @@ namespace DebugServer::Gdb virtual ~DebugSession(); virtual void setInternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ); virtual void removeInternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ); virtual void setExternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ); virtual void removeExternalBreakpoint( - const Targets::TargetBreakpoint& breakpoint, + Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ); diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/ClearHardwareBreakpoint.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/ClearHardwareBreakpoint.hpp new file mode 100644 index 00000000..df1ca1f4 --- /dev/null +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/ClearHardwareBreakpoint.hpp @@ -0,0 +1,30 @@ +#pragma once + +#include +#include + +#include "Avr8GenericCommandFrame.hpp" + +namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr::CommandFrames::Avr8Generic +{ + class ClearHardwareBreakpoint: public Avr8GenericCommandFrame> + { + public: + explicit ClearHardwareBreakpoint(std::uint8_t number) + : Avr8GenericCommandFrame() + { + /* + * The clear hardware breakpoint command consists of 3 bytes: + * + * 1. Command ID (0x41) + * 2. Version (0x00) + * 3. Breakpoint Number (1, 2, or 3) + */ + this->payload = { + 0x41, + 0x00, + number + }; + } + }; +} diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/SetHardwareBreakpoint.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/SetHardwareBreakpoint.hpp new file mode 100644 index 00000000..d2020960 --- /dev/null +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/SetHardwareBreakpoint.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include +#include + +#include "Avr8GenericCommandFrame.hpp" + +namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr::CommandFrames::Avr8Generic +{ + class SetHardwareBreakpoint: public Avr8GenericCommandFrame> + { + public: + explicit SetHardwareBreakpoint(std::uint32_t address, std::uint8_t number) + : Avr8GenericCommandFrame() + { + /* + * The set hardware breakpoint command consists of 9 bytes: + * + * 1. Command ID (0x40) + * 2. Version (0x00) + * 3. Breakpoint Type (0x01 for program break) + * 4. Breakpoint Number (1, 2, or 3) + * 5. Program address (4 bytes) - the EDBG Protocol document states that this should be the word address, + * but that seems to be incorrect. The tool expects a byte address here. + * 5. Mode (0x03 for program break) + */ + this->payload = { + 0x40, + 0x00, + 0x01, + number, + static_cast(address), + static_cast(address >> 8), + static_cast(address >> 16), + static_cast(address >> 24), + 0x03 + }; + } + }; +} diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp index 40d35ea4..a387ea5c 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp @@ -34,6 +34,8 @@ #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/SetSoftwareBreakpoints.hpp" #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/ClearAllSoftwareBreakpoints.hpp" #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/ClearSoftwareBreakpoints.hpp" +#include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/SetHardwareBreakpoint.hpp" +#include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/ClearHardwareBreakpoint.hpp" #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/EnterProgrammingMode.hpp" #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/LeaveProgrammingMode.hpp" #include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/CommandFrames/AVR8Generic/EraseMemory.hpp" @@ -58,6 +60,8 @@ namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr using CommandFrames::Avr8Generic::SetSoftwareBreakpoints; using CommandFrames::Avr8Generic::ClearSoftwareBreakpoints; using CommandFrames::Avr8Generic::ClearAllSoftwareBreakpoints; + using CommandFrames::Avr8Generic::SetHardwareBreakpoint; + using CommandFrames::Avr8Generic::ClearHardwareBreakpoint; using CommandFrames::Avr8Generic::ReadMemory; using CommandFrames::Avr8Generic::EnterProgrammingMode; using CommandFrames::Avr8Generic::LeaveProgrammingMode; @@ -346,7 +350,7 @@ namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr return responseFrame.extractSignature(this->targetConfig.physicalInterface); } - void EdbgAvr8Interface::setBreakpoint(TargetMemoryAddress address) { + void EdbgAvr8Interface::setSoftwareBreakpoint(TargetMemoryAddress address) { const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( SetSoftwareBreakpoints({address}) ); @@ -356,7 +360,7 @@ namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr } } - void EdbgAvr8Interface::clearBreakpoint(TargetMemoryAddress address) { + void EdbgAvr8Interface::clearSoftwareBreakpoint(TargetMemoryAddress address) { const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( ClearSoftwareBreakpoints({address}) ); @@ -366,6 +370,55 @@ namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr } } + 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; + }; + + 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.insert(std::pair(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::clearAllBreakpoints() { const auto responseFrame = this->edbgInterface->sendAvrCommandFrameAndWaitForResponseFrame( ClearAllSoftwareBreakpoints() diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp index f0fd555d..295a33bd 100644 --- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp @@ -164,18 +164,36 @@ namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr * byte address. * * @param address - * The byte address to position the breakpoint. + * The byte address to place the breakpoint. */ - void setBreakpoint(Targets::TargetMemoryAddress address) override; + void setSoftwareBreakpoint(Targets::TargetMemoryAddress address) override; /** - * Issues the "Software Breakpoint Clear" command to the debug tool, clearing any breakpoint at the given - * byte address. + * 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 clearBreakpoint(Targets::TargetMemoryAddress address) override; + 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; /** * Issues the "Software Breakpoint Clear All" command to the debug tool, clearing all software breakpoints @@ -345,6 +363,12 @@ namespace DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr bool programmingModeEnabled = false; + /** + * Every hardware breakpoint is assigned a "breakpoint number", which we need to keep track of in order to + * clear a hardware breakpoint. + */ + std::map hardwareBreakpointNumbersByAddress; + /** * Sends the necessary target parameters to the debug tool. * diff --git a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp index 2c4f8300..2f90805f 100644 --- a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR/AVR8/Avr8DebugInterface.hpp @@ -100,14 +100,28 @@ namespace DebugToolDrivers::TargetInterfaces::Microchip::Avr::Avr8 * * @param address */ - virtual void setBreakpoint(Targets::TargetMemoryAddress address) = 0; + virtual void setSoftwareBreakpoint(Targets::TargetMemoryAddress address) = 0; /** * Should remove a software breakpoint at a given address. * * @param address */ - virtual void clearBreakpoint(Targets::TargetMemoryAddress address) = 0; + virtual void clearSoftwareBreakpoint(Targets::TargetMemoryAddress address) = 0; + + /** + * Should set a hardware breakpoint at a given address. + * + * @param address + */ + virtual void setHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; + + /** + * Should remove a hardware breakpoint at a given address. + * + * @param address + */ + virtual void clearHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; /** * Should remove all software and hardware breakpoints on the target. diff --git a/src/ProjectConfig.cpp b/src/ProjectConfig.cpp index 503720b0..935d6142 100644 --- a/src/ProjectConfig.cpp +++ b/src/ProjectConfig.cpp @@ -134,6 +134,10 @@ TargetConfig::TargetConfig(const YAML::Node& targetNode) { this->variantName = StringService::asciiToLower(targetNode["variantName"].as()); } + if (targetNode["hardwareBreakpoints"]) { + this->hardwareBreakpoints = targetNode["hardwareBreakpoints"].as(this->hardwareBreakpoints); + } + this->targetNode = targetNode; } diff --git a/src/ProjectConfig.hpp b/src/ProjectConfig.hpp index 32f5f4b0..afc7297b 100644 --- a/src/ProjectConfig.hpp +++ b/src/ProjectConfig.hpp @@ -51,6 +51,11 @@ struct TargetConfig */ std::optional variantName; + /** + * Determines whether Bloom will make use of the target's hardware breakpoint resources (if available). + */ + bool hardwareBreakpoints = true; + /** * For extracting any target specific configuration. See Avr8TargetConfig::Avr8TargetConfig() and * Avr8::preActivationConfigure() for an example of this. diff --git a/src/Services/TargetControllerService.cpp b/src/Services/TargetControllerService.cpp index 054ea66a..dbed3525 100644 --- a/src/Services/TargetControllerService.cpp +++ b/src/Services/TargetControllerService.cpp @@ -218,12 +218,15 @@ namespace Services ); } - void TargetControllerService::setBreakpoint(TargetBreakpoint breakpoint) const { - this->commandManager.sendCommandAndWaitForResponse( - std::make_unique(breakpoint), + Targets::TargetBreakpoint TargetControllerService::setBreakpoint( + Targets::TargetMemoryAddress address, + Targets::TargetBreakpoint::Type preferredType + ) const { + return this->commandManager.sendCommandAndWaitForResponse( + std::make_unique(address, preferredType), this->defaultTimeout, this->activeAtomicSessionId - ); + )->breakpoint; } void TargetControllerService::removeBreakpoint(TargetBreakpoint breakpoint) const { diff --git a/src/Services/TargetControllerService.hpp b/src/Services/TargetControllerService.hpp index 2a49c388..116d7983 100644 --- a/src/Services/TargetControllerService.hpp +++ b/src/Services/TargetControllerService.hpp @@ -142,9 +142,16 @@ namespace Services /** * Requests the TargetController to set a breakpoint on the target. * - * @param breakpoint + * @param address + * @param preferredType + * + * @return + * The installed breakpoint. */ - void setBreakpoint(Targets::TargetBreakpoint breakpoint) const; + Targets::TargetBreakpoint setBreakpoint( + Targets::TargetMemoryAddress address, + Targets::TargetBreakpoint::Type preferredType = Targets::TargetBreakpoint::Type::HARDWARE + ) const; /** * Requests the TargetController to remove a breakpoint from the target. diff --git a/src/TargetController/Commands/SetBreakpoint.hpp b/src/TargetController/Commands/SetBreakpoint.hpp index 37029678..e4c8d863 100644 --- a/src/TargetController/Commands/SetBreakpoint.hpp +++ b/src/TargetController/Commands/SetBreakpoint.hpp @@ -1,22 +1,39 @@ #pragma once #include "Command.hpp" +#include "src/TargetController/Responses/Breakpoint.hpp" #include "src/Targets/TargetBreakpoint.hpp" +#include "src/Targets/TargetMemory.hpp" namespace TargetController::Commands { class SetBreakpoint: public Command { public: + using SuccessResponseType = Responses::Breakpoint; + static constexpr CommandType type = CommandType::SET_BREAKPOINT; static const inline std::string name = "SetBreakpoint"; - Targets::TargetBreakpoint breakpoint; + /** + * Byte address in program memory. + */ + Targets::TargetMemoryAddress address; - SetBreakpoint() = default; - explicit SetBreakpoint(const Targets::TargetBreakpoint& breakpoint) - : breakpoint(breakpoint) + /** + * The preferred breakpoint type (HARDWARE/SOFTWARE). + * + * There is no guarantee that the TC will be able to allocate resources for the preferred type. + * If the preferredType is set to HARDWARE, but the target doesn't have any available resources, or hardware + * breakpoints have been disabled (@see TargetConfig::hardwareBreakpoints), the TC will fall back to software + * breakpoints. + */ + Targets::TargetBreakpoint::Type preferredType; + + SetBreakpoint(Targets::TargetMemoryAddress address, Targets::TargetBreakpoint::Type preferredType) + : address(address) + , preferredType(preferredType) {}; [[nodiscard]] CommandType getType() const override { diff --git a/src/TargetController/Responses/Breakpoint.hpp b/src/TargetController/Responses/Breakpoint.hpp new file mode 100644 index 00000000..6ab52166 --- /dev/null +++ b/src/TargetController/Responses/Breakpoint.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include "Response.hpp" + +#include "src/Targets/TargetBreakpoint.hpp" + +namespace TargetController::Responses +{ + class Breakpoint: public Response + { + public: + static constexpr ResponseType type = ResponseType::BREAKPOINT; + + Targets::TargetBreakpoint breakpoint; + + explicit Breakpoint(const Targets::TargetBreakpoint& breakpoint) + : breakpoint(breakpoint) + {} + + [[nodiscard]] ResponseType getType() const override { + return Breakpoint::type; + } + }; +} diff --git a/src/TargetController/Responses/ResponseTypes.hpp b/src/TargetController/Responses/ResponseTypes.hpp index f48bc0e2..a743cd63 100644 --- a/src/TargetController/Responses/ResponseTypes.hpp +++ b/src/TargetController/Responses/ResponseTypes.hpp @@ -16,5 +16,6 @@ namespace TargetController::Responses TARGET_PIN_STATES, TARGET_STACK_POINTER, TARGET_PROGRAM_COUNTER, + BREAKPOINT, }; } diff --git a/src/TargetController/TargetControllerComponent.cpp b/src/TargetController/TargetControllerComponent.cpp index 8a2034f5..8638e3b7 100644 --- a/src/TargetController/TargetControllerComponent.cpp +++ b/src/TargetController/TargetControllerComponent.cpp @@ -7,6 +7,7 @@ #include "Responses/Error.hpp" #include "src/Services/ProcessService.hpp" +#include "src/Services/StringService.hpp" #include "src/Logger/Logger.hpp" #include "src/Exceptions/InvalidConfig.hpp" @@ -51,6 +52,7 @@ namespace TargetController using Responses::TargetPinStates; using Responses::TargetStackPointer; using Responses::TargetProgramCounter; + using Responses::Breakpoint; TargetControllerComponent::TargetControllerComponent( const ProjectConfig& projectConfig, @@ -504,6 +506,17 @@ namespace TargetController Logger::info("Target ID: " + targetDescriptor.id); Logger::info("Target name: " + targetDescriptor.name); + + if (!this->environmentConfig.targetConfig.hardwareBreakpoints) { + Logger::warning("Hardware breakpoints have been disabled"); + + } else if (targetDescriptor.breakpointResources.maximumHardwareBreakpoints.has_value()) { + Logger::info( + "Available hardware breakpoints: " + std::to_string( + *(targetDescriptor.breakpointResources.maximumHardwareBreakpoints) + ) + ); + } } void TargetControllerComponent::releaseHardware() { @@ -877,13 +890,76 @@ namespace TargetController return std::make_unique(); } - std::unique_ptr TargetControllerComponent::handleSetBreakpoint(SetBreakpoint& command) { - this->target->setBreakpoint(command.breakpoint.address); - return std::make_unique(); + std::unique_ptr TargetControllerComponent::handleSetBreakpoint(SetBreakpoint& command) { + using Targets::TargetBreakpoint; + using Services::StringService; + + auto breakpoint = TargetBreakpoint(command.address, TargetBreakpoint::Type::SOFTWARE); + + const auto& targetBreakpointResources = this->getTargetDescriptor().breakpointResources; + if ( + command.preferredType == Targets::TargetBreakpoint::Type::HARDWARE + && this->environmentConfig.targetConfig.hardwareBreakpoints + ) { + static auto exhaustedResourcesWarning = false; + + if ( + !targetBreakpointResources.maximumHardwareBreakpoints.has_value() + || this->hardwareBreakpointsByAddress.size() < *(targetBreakpointResources.maximumHardwareBreakpoints) + ) { + exhaustedResourcesWarning = true; + + Logger::debug( + "Installing hardware breakpoint at byte address 0x" + StringService::toHex(command.address) + ); + + this->target->setHardwareBreakpoint(command.address); + this->hardwareBreakpointsByAddress.insert(std::pair(command.address, breakpoint)); + + breakpoint.type = TargetBreakpoint::Type::HARDWARE; + return std::make_unique(breakpoint); + } + + if (exhaustedResourcesWarning) { + exhaustedResourcesWarning = false; + Logger::warning( + "Hardware breakpoint resources have been exhausted. Falling back to software breakpoints" + ); + } + } + + Logger::debug( + "Installing software breakpoint at byte address 0x" + StringService::toHex(command.address) + ); + + this->target->setSoftwareBreakpoint(command.address); + this->softwareBreakpointsByAddress.insert(std::pair(command.address, breakpoint)); + + return std::make_unique(breakpoint); } std::unique_ptr TargetControllerComponent::handleRemoveBreakpoint(RemoveBreakpoint& command) { - this->target->removeBreakpoint(command.breakpoint.address); + using Services::StringService; + + if (command.breakpoint.type == Targets::TargetBreakpoint::Type::HARDWARE) { + assert(this->environmentConfig.targetConfig.hardwareBreakpoints); + + Logger::debug( + "Removing hardware breakpoint at byte address 0x" + StringService::toHex(command.breakpoint.address) + ); + + this->target->removeHardwareBreakpoint(command.breakpoint.address); + this->hardwareBreakpointsByAddress.erase(command.breakpoint.address); + + } else { + Logger::debug( + "Removing software breakpoint at byte address 0x" + StringService::toHex(command.breakpoint.address) + ); + + this->target->removeSoftwareBreakpoint(command.breakpoint.address); + this->softwareBreakpointsByAddress.erase(command.breakpoint.address); + } + return std::make_unique(); } diff --git a/src/TargetController/TargetControllerComponent.hpp b/src/TargetController/TargetControllerComponent.hpp index 00b4bffe..7504e9c4 100644 --- a/src/TargetController/TargetControllerComponent.hpp +++ b/src/TargetController/TargetControllerComponent.hpp @@ -55,6 +55,7 @@ #include "Responses/TargetPinStates.hpp" #include "Responses/TargetStackPointer.hpp" #include "Responses/TargetProgramCounter.hpp" +#include "Responses/Breakpoint.hpp" #include "src/DebugToolDrivers/DebugTools.hpp" #include "src/Targets/Target.hpp" @@ -161,6 +162,12 @@ namespace TargetController */ std::map registerAddressRangeByMemoryType; + /** + * The TargetController keeps track of all installed breakpoints. + */ + std::map softwareBreakpointsByAddress; + std::map hardwareBreakpointsByAddress; + /** * Registers a handler function for a particular command type. * Only one handler function can be registered per command type. @@ -339,7 +346,7 @@ namespace TargetController std::unique_ptr handleWriteTargetMemory(Commands::WriteTargetMemory& command); std::unique_ptr handleEraseTargetMemory(Commands::EraseTargetMemory& command); std::unique_ptr handleStepTargetExecution(Commands::StepTargetExecution& command); - std::unique_ptr handleSetBreakpoint(Commands::SetBreakpoint& command); + std::unique_ptr handleSetBreakpoint(Commands::SetBreakpoint& command); std::unique_ptr handleRemoveBreakpoint(Commands::RemoveBreakpoint& command); std::unique_ptr handleSetProgramCounter(Commands::SetTargetProgramCounter& command); std::unique_ptr handleGetTargetPinStates(Commands::GetTargetPinStates& command); diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp index a956773f..5d5c21d5 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.cpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.cpp @@ -268,6 +268,7 @@ namespace Targets::Microchip::Avr::Avr8Bit "Microchip", this->targetMemoryDescriptorsByType, this->targetRegisterDescriptorsById, + this->getBreakpointResources(), {}, Targets::TargetMemoryType::FLASH ); @@ -304,12 +305,20 @@ namespace Targets::Microchip::Avr::Avr8Bit this->avr8DebugInterface->reset(); } - void Avr8::setBreakpoint(std::uint32_t address) { - this->avr8DebugInterface->setBreakpoint(address); + void Avr8::setSoftwareBreakpoint(TargetMemoryAddress address) { + this->avr8DebugInterface->setSoftwareBreakpoint(address); } - void Avr8::removeBreakpoint(std::uint32_t address) { - this->avr8DebugInterface->clearBreakpoint(address); + void Avr8::removeSoftwareBreakpoint(TargetMemoryAddress address) { + this->avr8DebugInterface->clearSoftwareBreakpoint(address); + } + + void Avr8::setHardwareBreakpoint(TargetMemoryAddress address) { + this->avr8DebugInterface->setHardwareBreakpoint(address); + } + + void Avr8::removeHardwareBreakpoint(TargetMemoryAddress address) { + this->avr8DebugInterface->clearHardwareBreakpoint(address); } void Avr8::clearAllBreakpoints() { @@ -687,6 +696,33 @@ namespace Targets::Microchip::Avr::Avr8Bit } } + BreakpointResources Avr8::getBreakpointResources() { + auto maxHardwareBreakpoints = 0; + + switch (this->targetConfig.physicalInterface) { + case PhysicalInterface::JTAG: { + maxHardwareBreakpoints = this->family == Family::XMEGA ? 2 : 3; + break; + } + case PhysicalInterface::PDI: { + maxHardwareBreakpoints = 2; + break; + } + case PhysicalInterface::UPDI: { + maxHardwareBreakpoints = 1; + break; + } + default: { + break; + } + } + + return BreakpointResources( + maxHardwareBreakpoints, + std::nullopt + ); + } + bool Avr8::isFuseEnabled(const FuseBitsDescriptor& descriptor, unsigned char fuseByteValue) const { const auto programmedValue = static_cast( this->fuseEnableStrategy == FuseEnableStrategy::SET diff --git a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp index 69b911a7..bfd1e22f 100644 --- a/src/Targets/Microchip/AVR/AVR8/Avr8.hpp +++ b/src/Targets/Microchip/AVR/AVR8/Avr8.hpp @@ -19,6 +19,7 @@ #include "src/Targets/Microchip/AVR/Fuse.hpp" #include "src/Targets/TargetRegister.hpp" +#include "src/Targets/TargetBreakpoint.hpp" #include "TargetDescription/TargetDescriptionFile.hpp" @@ -58,8 +59,11 @@ namespace Targets::Microchip::Avr::Avr8Bit void step() override; void reset() override; - void setBreakpoint(TargetProgramCounter address) override; - void removeBreakpoint(TargetProgramCounter address) override; + void setSoftwareBreakpoint(TargetProgramCounter address) override; + void removeSoftwareBreakpoint(TargetProgramCounter address) override; + + void setHardwareBreakpoint(TargetProgramCounter address) override; + void removeHardwareBreakpoint(TargetProgramCounter address) override; void clearAllBreakpoints() override; void writeRegisters(TargetRegisters registers) override; @@ -143,6 +147,8 @@ namespace Targets::Microchip::Avr::Avr8Bit void loadTargetMemoryDescriptors(); + BreakpointResources getBreakpointResources(); + /** * Checks if a particular fuse is enabled in the given fuse byte value. Takes the target's fuse enable strategy * into account. diff --git a/src/Targets/Target.hpp b/src/Targets/Target.hpp index b6e7665b..53835aad 100644 --- a/src/Targets/Target.hpp +++ b/src/Targets/Target.hpp @@ -111,18 +111,32 @@ namespace Targets virtual void reset() = 0; /** - * Should set a breakpoint on the target, at the given address. + * Should set a software breakpoint on the target, at the given address. * * @param address */ - virtual void setBreakpoint(TargetMemoryAddress address) = 0; + virtual void setSoftwareBreakpoint(TargetMemoryAddress address) = 0; /** - * Should remove a breakpoint at the given address. + * Should remove a software breakpoint at the given address. * * @param address */ - virtual void removeBreakpoint(TargetMemoryAddress address) = 0; + virtual void removeSoftwareBreakpoint(TargetMemoryAddress address) = 0; + + /** + * Should set a hardware breakpoint on the target, at the given address. + * + * @param address + */ + virtual void setHardwareBreakpoint(TargetMemoryAddress address) = 0; + + /** + * Should remove a hardware breakpoint at the given address. + * + * @param address + */ + virtual void removeHardwareBreakpoint(TargetMemoryAddress address) = 0; /** * Should clear all breakpoints on the target. diff --git a/src/Targets/TargetBreakpoint.hpp b/src/Targets/TargetBreakpoint.hpp index 546e0b55..ee9da87d 100644 --- a/src/Targets/TargetBreakpoint.hpp +++ b/src/Targets/TargetBreakpoint.hpp @@ -1,12 +1,13 @@ #pragma once #include +#include #include "TargetMemory.hpp" namespace Targets { - enum class TargetBreakCause: int + enum class TargetBreakCause: std::uint8_t { BREAKPOINT, UNKNOWN, @@ -14,12 +15,38 @@ namespace Targets struct TargetBreakpoint { + enum class Type: std::uint8_t + { + HARDWARE, + SOFTWARE, + }; + /** * Byte address of the breakpoint. */ TargetMemoryAddress address = 0; + Type type = Type::SOFTWARE; + TargetBreakpoint() = default; - explicit TargetBreakpoint(TargetMemoryAddress address): address(address) {}; + + explicit TargetBreakpoint(TargetMemoryAddress address, Type type = Type::SOFTWARE) + : address(address) + , type(type) + {}; + }; + + struct BreakpointResources + { + std::optional maximumHardwareBreakpoints; + std::optional maximumSoftwareBreakpoints; + + BreakpointResources( + std::optional maximumHardwareBreakpoints, + std::optional maximumSoftwareBreakpoints + ) + : maximumHardwareBreakpoints(maximumHardwareBreakpoints) + , maximumSoftwareBreakpoints(maximumSoftwareBreakpoints) + {} }; } diff --git a/src/Targets/TargetDescriptor.hpp b/src/Targets/TargetDescriptor.hpp index b9c624e3..02e88f1f 100644 --- a/src/Targets/TargetDescriptor.hpp +++ b/src/Targets/TargetDescriptor.hpp @@ -10,6 +10,7 @@ #include "TargetMemory.hpp" #include "TargetRegister.hpp" #include "TargetVariant.hpp" +#include "TargetBreakpoint.hpp" namespace Targets { @@ -20,6 +21,7 @@ namespace Targets std::string vendorName; std::map memoryDescriptorsByType; std::map registerDescriptorsById; + BreakpointResources breakpointResources; std::vector variants; TargetMemoryType programMemoryType; @@ -30,6 +32,7 @@ namespace Targets const std::string& vendorName, const std::map& memoryDescriptorsByType, const std::map& registerDescriptorsById, + const BreakpointResources& breakpointResources, const std::vector& variants, TargetMemoryType programMemoryType ) @@ -38,6 +41,7 @@ namespace Targets , vendorName(vendorName) , memoryDescriptorsByType(memoryDescriptorsByType) , registerDescriptorsById(registerDescriptorsById) + , breakpointResources(breakpointResources) , variants(variants) , programMemoryType(programMemoryType) {}