diff --git a/src/DebugServer/CMakeLists.txt b/src/DebugServer/CMakeLists.txt index 20099a94..b9483c63 100755 --- a/src/DebugServer/CMakeLists.txt +++ b/src/DebugServer/CMakeLists.txt @@ -14,8 +14,6 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/ContinueExecution.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/StepExecution.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/InterruptExecution.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/SetBreakpoint.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/RemoveBreakpoint.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/Monitor.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/ResetTarget.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/CommandPackets/HelpMonitorInfo.cpp @@ -37,6 +35,8 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/ReadMemoryMap.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/SetBreakpoint.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/RemoveBreakpoint.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/FlashErase.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/FlashWrite.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/AvrGdb/CommandPackets/FlashDone.cpp @@ -53,6 +53,8 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/ReadMemory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/WriteMemory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/ReadMemoryMap.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/RemoveBreakpoint.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/FlashErase.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/FlashWrite.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Gdb/RiscVGdb/CommandPackets/FlashDone.cpp diff --git a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp index e54da159..1a3dd373 100644 --- a/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp +++ b/src/DebugServer/Gdb/AvrGdb/AvrGdbRsp.cpp @@ -9,6 +9,8 @@ #include "CommandPackets/ReadMemory.hpp" #include "CommandPackets/WriteMemory.hpp" #include "CommandPackets/ReadMemoryMap.hpp" +#include "CommandPackets/SetBreakpoint.hpp" +#include "CommandPackets/RemoveBreakpoint.hpp" #include "CommandPackets/FlashErase.hpp" #include "CommandPackets/FlashWrite.hpp" #include "CommandPackets/FlashDone.hpp" @@ -56,6 +58,8 @@ namespace DebugServer::Gdb::AvrGdb using CommandPackets::ReadMemory; using CommandPackets::WriteMemory; using CommandPackets::ReadMemoryMap; + using CommandPackets::SetBreakpoint; + using CommandPackets::RemoveBreakpoint; using CommandPackets::FlashErase; using CommandPackets::FlashWrite; using CommandPackets::FlashDone; @@ -87,6 +91,14 @@ namespace DebugServer::Gdb::AvrGdb return std::make_unique(rawPacket, this->gdbTargetDescriptor); } + if (rawPacket[1] == 'Z') { + return std::make_unique(rawPacket); + } + + if (rawPacket[1] == 'z') { + return std::make_unique(rawPacket); + } + if (rawPacket.size() > 1) { const auto rawPacketString = std::string{rawPacket.begin() + 1, rawPacket.end()}; @@ -161,7 +173,12 @@ namespace DebugServer::Gdb::AvrGdb * * We need to figure out why, and determine whether the stop should be reported to GDB. */ - if (this->debugSession->externalBreakpointsByAddress.contains(programAddress)) { + if ( + this->debugSession->externalBreakpointRegistry.contains( + activeRangeSteppingSession->addressSpaceDescriptor.id, + programAddress + ) + ) { /* * The target stopped due to an external breakpoint, set by GDB. * diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/RemoveBreakpoint.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/RemoveBreakpoint.cpp new file mode 100644 index 00000000..c27f0a1c --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/RemoveBreakpoint.cpp @@ -0,0 +1,76 @@ +#include "RemoveBreakpoint.hpp" + +#include + +#include "src/DebugServer/Gdb/ResponsePackets/OkResponsePacket.hpp" +#include "src/DebugServer/Gdb/ResponsePackets/EmptyResponsePacket.hpp" +#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" + +#include "src/Targets/TargetBreakpoint.hpp" + +#include "src/Services/StringService.hpp" + +#include "src/Logger/Logger.hpp" +#include "src/Exceptions/Exception.hpp" + +namespace DebugServer::Gdb::AvrGdb::CommandPackets +{ + using Services::TargetControllerService; + + using ResponsePackets::OkResponsePacket; + using ResponsePackets::ErrorResponsePacket; + using ResponsePackets::EmptyResponsePacket; + + using ::Exceptions::Exception; + + RemoveBreakpoint::RemoveBreakpoint(const RawPacket& rawPacket) + : CommandPacket(rawPacket) + { + const auto packetString = std::string{this->data.begin(), this->data.end()}; + + if (packetString.size() < 6) { + throw Exception{"Unexpected RemoveBreakpoint packet size"}; + } + + this->type = (packetString[1] == '0') + ? BreakpointType::SOFTWARE_BREAKPOINT + : (packetString[1] == '1') ? BreakpointType::HARDWARE_BREAKPOINT : BreakpointType::UNKNOWN; + + const auto firstCommaPos = packetString.find_first_of(','); + const auto secondCommaPos = packetString.find_first_of(',', firstCommaPos + 1); + if (firstCommaPos == std::string::npos || secondCommaPos == std::string::npos) { + throw Exception{"Invalid RemoveBreakpoint packet"}; + } + + const auto addressString = packetString.substr(firstCommaPos + 1, secondCommaPos - firstCommaPos - 1); + const auto sizeString = packetString.substr( + secondCommaPos + 1, + packetString.find_first_of(';', secondCommaPos) - secondCommaPos - 1 + ); + + this->address = Services::StringService::toUint32(addressString, 16); + this->size = Services::StringService::toUint32(sizeString, 10); + } + + void RemoveBreakpoint::handle( + DebugSession& debugSession, + const AvrGdbTargetDescriptor& gdbTargetDescriptor, + const Targets::TargetDescriptor&, + TargetControllerService& targetControllerService + ) { + Logger::info("Handling RemoveBreakpoint packet"); + + try { + debugSession.removeExternalBreakpoint( + gdbTargetDescriptor.programAddressSpaceDescriptor, + this->address, + targetControllerService + ); + debugSession.connection.writePacket(OkResponsePacket{}); + + } catch (const Exception& exception) { + Logger::error("Failed to remove breakpoint on target - " + exception.getMessage()); + debugSession.connection.writePacket(ErrorResponsePacket{}); + } + } +} diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/RemoveBreakpoint.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/RemoveBreakpoint.hpp new file mode 100644 index 00000000..73d461ac --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/RemoveBreakpoint.hpp @@ -0,0 +1,42 @@ +#pragma once + +#include + +#include "AvrGdbCommandPacketInterface.hpp" +#include "src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp" +#include "src/DebugServer/Gdb/BreakpointType.hpp" + +#include "src/Targets/TargetMemory.hpp" + +namespace DebugServer::Gdb::AvrGdb::CommandPackets +{ + /** + * The RemoveBreakpoint class implements the structure for "Z" command packets. Upon receiving this command, the + * server is expected to set a breakpoint at the specified address. + */ + class RemoveBreakpoint + : public CommandPackets::AvrGdbCommandPacketInterface + , private Gdb::CommandPackets::CommandPacket + { + public: + /** + * The requested breakpoint type. + * + * We don't actually honor this. GDB assumes flash memory cannot be written to outside of a programming + * session, so it refuses to even attempt to insert software breakpoints in flash memory. It always requests + * hardware breakpoints. This is why ignore the requested breakpoint type and just insert whatever type we can. + */ + BreakpointType type = BreakpointType::UNKNOWN; + Targets::TargetMemoryAddress address = 0; + Targets::TargetMemorySize size = 0; + + explicit RemoveBreakpoint(const RawPacket& rawPacket); + + void handle( + DebugSession& debugSession, + const AvrGdbTargetDescriptor& gdbTargetDescriptor, + const Targets::TargetDescriptor& targetDescriptor, + Services::TargetControllerService& targetControllerService + ) override; + }; +} diff --git a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/SetBreakpoint.cpp similarity index 55% rename from src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp rename to src/DebugServer/Gdb/AvrGdb/CommandPackets/SetBreakpoint.cpp index f53bdddd..1972c790 100644 --- a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/SetBreakpoint.cpp @@ -1,6 +1,6 @@ #include "SetBreakpoint.hpp" -#include +#include #include "src/DebugServer/Gdb/ResponsePackets/OkResponsePacket.hpp" #include "src/DebugServer/Gdb/ResponsePackets/EmptyResponsePacket.hpp" @@ -8,15 +8,15 @@ #include "src/Targets/TargetBreakpoint.hpp" +#include "src/Services/StringService.hpp" + #include "src/Logger/Logger.hpp" #include "src/Exceptions/Exception.hpp" -namespace DebugServer::Gdb::CommandPackets +namespace DebugServer::Gdb::AvrGdb::CommandPackets { using Services::TargetControllerService; - using Targets::TargetBreakpoint; - using ResponsePackets::OkResponsePacket; using ResponsePackets::ErrorResponsePacket; using ResponsePackets::EmptyResponsePacket; @@ -26,36 +26,36 @@ namespace DebugServer::Gdb::CommandPackets SetBreakpoint::SetBreakpoint(const RawPacket& rawPacket) : CommandPacket(rawPacket) { - if (this->data.size() < 6) { + const auto packetString = std::string{this->data.begin(), this->data.end()}; + + if (packetString.size() < 6) { throw Exception{"Unexpected SetBreakpoint packet size"}; } // Z0 = SW breakpoint, Z1 = HW breakpoint - this->type = (this->data[1] == '0') + this->type = (packetString[1] == '0') ? BreakpointType::SOFTWARE_BREAKPOINT - : (this->data[1] == '1') ? BreakpointType::HARDWARE_BREAKPOINT : BreakpointType::UNKNOWN; + : (packetString[1] == '1') ? BreakpointType::HARDWARE_BREAKPOINT : BreakpointType::UNKNOWN; - auto packetData = QString::fromLocal8Bit( - reinterpret_cast(this->data.data() + 2), - static_cast(this->data.size() - 2) + const auto firstCommaPos = packetString.find_first_of(','); + const auto secondCommaPos = packetString.find_first_of(',', firstCommaPos + 1); + if (firstCommaPos == std::string::npos || secondCommaPos == std::string::npos) { + throw Exception{"Invalid SetBreakpoint packet"}; + } + + const auto addressString = packetString.substr(firstCommaPos + 1, secondCommaPos - firstCommaPos - 1); + const auto sizeString = packetString.substr( + secondCommaPos + 1, + packetString.find_first_of(';', secondCommaPos) - secondCommaPos - 1 ); - auto packetSegments = packetData.split(","); - if (packetSegments.size() < 3) { - throw Exception{"Unexpected number of packet segments in SetBreakpoint packet"}; - } - - bool conversionStatus = true; - this->address = packetSegments.at(1).toUInt(&conversionStatus, 16); - - if (!conversionStatus) { - throw Exception{"Failed to convert address hex value from SetBreakpoint packet."}; - } + this->address = Services::StringService::toUint32(addressString, 16); + this->size = Services::StringService::toUint32(sizeString, 10); } void SetBreakpoint::handle( DebugSession& debugSession, - const TargetDescriptor& gdbTargetDescriptor, + const AvrGdbTargetDescriptor& gdbTargetDescriptor, const Targets::TargetDescriptor&, TargetControllerService& targetControllerService ) { @@ -71,7 +71,11 @@ namespace DebugServer::Gdb::CommandPackets return; } - debugSession.setExternalBreakpoint(this->address, targetControllerService); + debugSession.setExternalBreakpoint( + gdbTargetDescriptor.programAddressSpaceDescriptor, + gdbTargetDescriptor.programMemorySegmentDescriptor, + this->address, this->size, targetControllerService + ); debugSession.connection.writePacket(OkResponsePacket{}); } catch (const Exception& exception) { diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/SetBreakpoint.hpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/SetBreakpoint.hpp new file mode 100644 index 00000000..fb6f7379 --- /dev/null +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/SetBreakpoint.hpp @@ -0,0 +1,31 @@ +#pragma once + +#include + +#include "AvrGdbCommandPacketInterface.hpp" +#include "src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp" +#include "src/DebugServer/Gdb/BreakpointType.hpp" + +#include "src/Targets/TargetMemory.hpp" + +namespace DebugServer::Gdb::AvrGdb::CommandPackets +{ + class SetBreakpoint + : public CommandPackets::AvrGdbCommandPacketInterface + , private Gdb::CommandPackets::CommandPacket + { + public: + BreakpointType type = BreakpointType::UNKNOWN; + Targets::TargetMemoryAddress address = 0; + Targets::TargetMemorySize size = 0; + + explicit SetBreakpoint(const RawPacket& rawPacket); + + void handle( + DebugSession& debugSession, + const AvrGdbTargetDescriptor& gdbTargetDescriptor, + const Targets::TargetDescriptor& targetDescriptor, + Services::TargetControllerService& targetControllerService + ) override; + }; +} diff --git a/src/DebugServer/Gdb/AvrGdb/CommandPackets/VContRangeStep.cpp b/src/DebugServer/Gdb/AvrGdb/CommandPackets/VContRangeStep.cpp index 917779bf..b2956c1d 100644 --- a/src/DebugServer/Gdb/AvrGdb/CommandPackets/VContRangeStep.cpp +++ b/src/DebugServer/Gdb/AvrGdb/CommandPackets/VContRangeStep.cpp @@ -84,7 +84,12 @@ namespace DebugServer::Gdb::AvrGdb::CommandPackets return; } - auto rangeSteppingSession = RangeSteppingSession{stepAddressRange, {}}; + auto rangeSteppingSession = RangeSteppingSession{ + gdbTargetDescriptor.programAddressSpaceDescriptor, + gdbTargetDescriptor.programMemorySegmentDescriptor, + stepAddressRange, + {} + }; const auto instructionsByAddress = Decoder::decode( stepAddressRange.startAddress, diff --git a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp b/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp deleted file mode 100644 index 871a545c..00000000 --- a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.cpp +++ /dev/null @@ -1,71 +0,0 @@ -#include "RemoveBreakpoint.hpp" - -#include - -#include "src/DebugServer/Gdb/ResponsePackets/OkResponsePacket.hpp" -#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" - -#include "src/Targets/TargetBreakpoint.hpp" - -#include "src/Logger/Logger.hpp" -#include "src/Exceptions/Exception.hpp" - -namespace DebugServer::Gdb::CommandPackets -{ - using Services::TargetControllerService; - - using Targets::TargetBreakpoint; - - using ResponsePackets::OkResponsePacket; - using ResponsePackets::ErrorResponsePacket; - - using ::Exceptions::Exception; - - RemoveBreakpoint::RemoveBreakpoint(const RawPacket& rawPacket) - : CommandPacket(rawPacket) - { - if (this->data.size() < 6) { - throw Exception{"Unexpected RemoveBreakpoint packet size"}; - } - - // z0 = SW breakpoint, z1 = HW breakpoint - this->type = (this->data[1] == '0') - ? BreakpointType::SOFTWARE_BREAKPOINT - : (this->data[1] == '1') ? BreakpointType::HARDWARE_BREAKPOINT : BreakpointType::UNKNOWN; - - const auto packetData = QString::fromLocal8Bit( - reinterpret_cast(this->data.data() + 2), - static_cast(this->data.size() - 2) - ); - - auto packetSegments = packetData.split(","); - if (packetSegments.size() < 3) { - throw Exception{"Unexpected number of packet segments in RemoveBreakpoint packet"}; - } - - bool conversionStatus = true; - this->address = packetSegments.at(1).toUInt(&conversionStatus, 16); - - if (!conversionStatus) { - throw Exception{"Failed to convert address hex value from RemoveBreakpoint packet."}; - } - } - - void RemoveBreakpoint::handle( - DebugSession& debugSession, - const TargetDescriptor& gdbTargetDescriptor, - const Targets::TargetDescriptor&, - TargetControllerService& targetControllerService - ) { - Logger::info("Handling RemoveBreakpoint packet"); - - try { - debugSession.removeExternalBreakpoint(this->address, targetControllerService); - debugSession.connection.writePacket(OkResponsePacket{}); - - } catch (const Exception& exception) { - Logger::error("Failed to remove breakpoint on target - " + exception.getMessage()); - debugSession.connection.writePacket(ErrorResponsePacket{}); - } - } -} diff --git a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.hpp b/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.hpp deleted file mode 100644 index 468424cf..00000000 --- a/src/DebugServer/Gdb/CommandPackets/RemoveBreakpoint.hpp +++ /dev/null @@ -1,40 +0,0 @@ -#pragma once - -#include -#include -#include - -#include "CommandPacket.hpp" -#include "src/DebugServer/Gdb/BreakpointType.hpp" - -#include "src/Targets/TargetMemory.hpp" - -namespace DebugServer::Gdb::CommandPackets -{ - /** - * The RemoveBreakpoint class implements the structure for "z" command packets. Upon receiving this command, the - * server is expected to remove a breakpoint at the specified address. - */ - class RemoveBreakpoint: public CommandPacket - { - public: - /** - * Breakpoint type (Software or Hardware) - */ - BreakpointType type = BreakpointType::UNKNOWN; - - /** - * Address at which the breakpoint should be located. - */ - Targets::TargetMemoryAddress address = 0; - - explicit RemoveBreakpoint(const RawPacket& rawPacket); - - void handle( - DebugSession& debugSession, - const TargetDescriptor& gdbTargetDescriptor, - const Targets::TargetDescriptor& targetDescriptor, - Services::TargetControllerService& targetControllerService - ) override; - }; -} diff --git a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.hpp b/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.hpp deleted file mode 100644 index a14437bc..00000000 --- a/src/DebugServer/Gdb/CommandPackets/SetBreakpoint.hpp +++ /dev/null @@ -1,40 +0,0 @@ -#pragma once - -#include -#include -#include - -#include "CommandPacket.hpp" -#include "src/DebugServer/Gdb/BreakpointType.hpp" - -#include "src/Targets/TargetMemory.hpp" - -namespace DebugServer::Gdb::CommandPackets -{ - /** - * The SetBreakpoint class implements the structure for "Z" command packets. Upon receiving this command, the - * server is expected to set a breakpoint at the specified address. - */ - class SetBreakpoint: public CommandPacket - { - public: - /** - * Breakpoint type (Software or Hardware) - */ - BreakpointType type = BreakpointType::UNKNOWN; - - /** - * Address at which the breakpoint should be located. - */ - Targets::TargetMemoryAddress address = 0; - - explicit SetBreakpoint(const RawPacket& rawPacket); - - void handle( - DebugSession& debugSession, - const TargetDescriptor& gdbTargetDescriptor, - const Targets::TargetDescriptor& targetDescriptor, - Services::TargetControllerService& targetControllerService - ) override; - }; -} diff --git a/src/DebugServer/Gdb/DebugSession.cpp b/src/DebugServer/Gdb/DebugSession.cpp index e9e88cea..203ad0e1 100644 --- a/src/DebugServer/Gdb/DebugSession.cpp +++ b/src/DebugServer/Gdb/DebugSession.cpp @@ -26,78 +26,93 @@ namespace DebugServer::Gdb } void DebugSession::setInternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, Targets::TargetMemoryAddress address, + Targets::TargetMemorySize size, Services::TargetControllerService& targetControllerService ) { - if (this->internalBreakpointsByAddress.contains(address)) { + if (this->internalBreakpointRegistry.contains(addressSpaceDescriptor.id, address)) { return; } - const auto externalBreakpointIt = this->externalBreakpointsByAddress.find(address); - if (externalBreakpointIt != this->externalBreakpointsByAddress.end()) { + const auto externalBreakpoint = this->externalBreakpointRegistry.find(addressSpaceDescriptor.id, address); + if (externalBreakpoint.has_value()) { // We already have an external breakpoint at this address - this->internalBreakpointsByAddress.emplace(address, externalBreakpointIt->second); + this->internalBreakpointRegistry.insert(externalBreakpoint->get()); return; } - this->internalBreakpointsByAddress.emplace( - address, - targetControllerService.setBreakpoint(address, Targets::TargetBreakpoint::Type::HARDWARE) + this->internalBreakpointRegistry.insert( + targetControllerService.setProgramBreakpointAnyType( + addressSpaceDescriptor, + memorySegmentDescriptor, + address, + size + ) ); } void DebugSession::removeInternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ) { - const auto breakpointIt = this->internalBreakpointsByAddress.find(address); - if (breakpointIt == this->internalBreakpointsByAddress.end()) { + const auto breakpoint = this->internalBreakpointRegistry.find(addressSpaceDescriptor.id, address); + if (!breakpoint.has_value()) { return; } - if (!this->externalBreakpointsByAddress.contains(address)) { - targetControllerService.removeBreakpoint(breakpointIt->second); + if (!this->externalBreakpointRegistry.contains(addressSpaceDescriptor.id, address)) { + targetControllerService.removeProgramBreakpoint(breakpoint->get()); } - this->internalBreakpointsByAddress.erase(breakpointIt); + this->internalBreakpointRegistry.remove(addressSpaceDescriptor.id, address); } void DebugSession::setExternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, Targets::TargetMemoryAddress address, + Targets::TargetMemorySize size, Services::TargetControllerService& targetControllerService ) { - if (this->externalBreakpointsByAddress.contains(address)) { + if (this->externalBreakpointRegistry.contains(addressSpaceDescriptor.id, address)) { return; } - const auto internalBreakpointIt = this->internalBreakpointsByAddress.find(address); - - if (internalBreakpointIt != this->internalBreakpointsByAddress.end()) { + const auto internalBreakpoint = this->internalBreakpointRegistry.find(addressSpaceDescriptor.id, address); + if (internalBreakpoint.has_value()) { // We already have an internal breakpoint at this address - this->externalBreakpointsByAddress.emplace(address, internalBreakpointIt->second); + this->externalBreakpointRegistry.insert(internalBreakpoint->get()); return; } - this->externalBreakpointsByAddress.emplace( - address, - targetControllerService.setBreakpoint(address, Targets::TargetBreakpoint::Type::HARDWARE) + this->externalBreakpointRegistry.insert( + targetControllerService.setProgramBreakpointAnyType( + addressSpaceDescriptor, + memorySegmentDescriptor, + address, + size + ) ); } void DebugSession::removeExternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ) { - const auto breakpointIt = this->externalBreakpointsByAddress.find(address); - if (breakpointIt == this->externalBreakpointsByAddress.end()) { + const auto breakpoint = this->externalBreakpointRegistry.find(addressSpaceDescriptor.id, address); + if (!breakpoint.has_value()) { return; } - if (!this->internalBreakpointsByAddress.contains(address)) { - targetControllerService.removeBreakpoint(breakpointIt->second); + if (!this->internalBreakpointRegistry.contains(addressSpaceDescriptor.id, address)) { + targetControllerService.removeProgramBreakpoint(breakpoint->get()); } - this->externalBreakpointsByAddress.erase(breakpointIt); + this->externalBreakpointRegistry.remove(addressSpaceDescriptor.id, address); } void DebugSession::startRangeSteppingSession( @@ -105,10 +120,22 @@ namespace DebugServer::Gdb Services::TargetControllerService& targetControllerService ) { for (const auto& interceptAddress : session.interceptedAddresses) { - this->setInternalBreakpoint(interceptAddress, targetControllerService); + /* + * Have hard-coded the breakpoint size here, as range stepping is only supported on AVR targets. + * + * TODO: Review this after v2.0.0. Maybe move this range-stepping code out to an AVR-specific DebugSession + * struct. Or, refactor the range stepping session object to accommodate breakpoint sizes. + */ + this->setInternalBreakpoint( + session.addressSpaceDescriptor, + session.memorySegmentDescriptor, + interceptAddress, + 2, + targetControllerService + ); } - this->activeRangeSteppingSession = std::move(session); + this->activeRangeSteppingSession.emplace(std::move(session)); } void DebugSession::terminateRangeSteppingSession(Services::TargetControllerService& targetControllerService) { @@ -118,7 +145,11 @@ namespace DebugServer::Gdb // Clear all intercepting breakpoints for (const auto& interceptAddress : this->activeRangeSteppingSession->interceptedAddresses) { - this->removeInternalBreakpoint(interceptAddress, targetControllerService); + this->removeInternalBreakpoint( + this->activeRangeSteppingSession->addressSpaceDescriptor, + interceptAddress, + targetControllerService + ); } this->activeRangeSteppingSession.reset(); diff --git a/src/DebugServer/Gdb/DebugSession.hpp b/src/DebugServer/Gdb/DebugSession.hpp index a5cf18d6..71a68027 100644 --- a/src/DebugServer/Gdb/DebugSession.hpp +++ b/src/DebugServer/Gdb/DebugSession.hpp @@ -12,7 +12,10 @@ #include "RangeSteppingSession.hpp" #include "src/Targets/TargetMemory.hpp" +#include "src/Targets/TargetAddressSpaceDescriptor.hpp" +#include "src/Targets/TargetMemorySegmentDescriptor.hpp" #include "src/Targets/TargetBreakpoint.hpp" +#include "src/Targets/ProgramBreakpointRegistry.hpp" #include "src/Services/TargetControllerService.hpp" @@ -53,8 +56,8 @@ namespace DebugServer::Gdb * * We track internal and external breakpoints separately. */ - std::map internalBreakpointsByAddress; - std::map externalBreakpointsByAddress; + Targets::ProgramBreakpointRegistry internalBreakpointRegistry; + Targets::ProgramBreakpointRegistry externalBreakpointRegistry; /** * When the GDB client is waiting for the target to halt, this is set to true so we know when to notify the @@ -102,21 +105,29 @@ namespace DebugServer::Gdb virtual ~DebugSession(); virtual void setInternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, Targets::TargetMemoryAddress address, + Targets::TargetMemorySize size, Services::TargetControllerService& targetControllerService ); virtual void removeInternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ); virtual void setExternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, Targets::TargetMemoryAddress address, + Targets::TargetMemorySize size, Services::TargetControllerService& targetControllerService ); virtual void removeExternalBreakpoint( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, Targets::TargetMemoryAddress address, Services::TargetControllerService& targetControllerService ); diff --git a/src/DebugServer/Gdb/GdbRspDebugServer.hpp b/src/DebugServer/Gdb/GdbRspDebugServer.hpp index 4e0e4957..6e5964bd 100644 --- a/src/DebugServer/Gdb/GdbRspDebugServer.hpp +++ b/src/DebugServer/Gdb/GdbRspDebugServer.hpp @@ -29,8 +29,6 @@ #include "CommandPackets/InterruptExecution.hpp" #include "CommandPackets/ContinueExecution.hpp" #include "CommandPackets/StepExecution.hpp" -#include "CommandPackets/SetBreakpoint.hpp" -#include "CommandPackets/RemoveBreakpoint.hpp" #include "CommandPackets/Monitor.hpp" #include "CommandPackets/ResetTarget.hpp" #include "CommandPackets/HelpMonitorInfo.hpp" @@ -332,6 +330,26 @@ namespace DebugServer::Gdb std::optional debugSession; void endDebugSession() { + if (!debugSession.has_value()) { + return; + } + + /* + * When GDB is configured to leave breakpoints in place, it will sometimes not even bother to remove them + * at the end of the debug session. + */ + for (const auto& [addressSpaceId, breakpointsByAddress] : this->debugSession->internalBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->targetControllerService.removeProgramBreakpoint(breakpoint); + } + } + + for (const auto& [addressSpaceId, breakpointsByAddress] : this->debugSession->externalBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->targetControllerService.removeProgramBreakpoint(breakpoint); + } + } + this->debugSession.reset(); } @@ -421,14 +439,6 @@ namespace DebugServer::Gdb return std::make_unique(rawPacket); } - if (rawPacket[1] == 'Z') { - return std::make_unique(rawPacket); - } - - if (rawPacket[1] == 'z') { - return std::make_unique(rawPacket); - } - if (rawPacket[1] == 'D') { return std::make_unique(rawPacket); } diff --git a/src/DebugServer/Gdb/RangeSteppingSession.hpp b/src/DebugServer/Gdb/RangeSteppingSession.hpp index fdb136df..ad457afc 100644 --- a/src/DebugServer/Gdb/RangeSteppingSession.hpp +++ b/src/DebugServer/Gdb/RangeSteppingSession.hpp @@ -3,6 +3,8 @@ #include #include "src/Targets/TargetMemory.hpp" +#include "src/Targets/TargetAddressSpaceDescriptor.hpp" +#include "src/Targets/TargetMemorySegmentDescriptor.hpp" namespace DebugServer::Gdb { @@ -15,6 +17,12 @@ namespace DebugServer::Gdb */ struct RangeSteppingSession { + /** + * The address space and memory segment of the program memory which we are stepping over. + */ + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor; + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor; + /** * The (byte) address range we're stepping over. * @@ -36,11 +44,23 @@ namespace DebugServer::Gdb bool singleStepping = false; RangeSteppingSession( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, const Targets::TargetMemoryAddressRange& range, const std::set& interceptedAddresses ) - : range(range) + : addressSpaceDescriptor(addressSpaceDescriptor) + , memorySegmentDescriptor(memorySegmentDescriptor) + , range(range) , interceptedAddresses(interceptedAddresses) {}; + + RangeSteppingSession(RangeSteppingSession&& other) noexcept + : addressSpaceDescriptor(other.addressSpaceDescriptor) + , memorySegmentDescriptor(other.memorySegmentDescriptor) + , range(other.range) + , interceptedAddresses(std::move(other.interceptedAddresses)) + , singleStepping(other.singleStepping) + {} }; } diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/RemoveBreakpoint.cpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/RemoveBreakpoint.cpp new file mode 100644 index 00000000..e20035ab --- /dev/null +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/RemoveBreakpoint.cpp @@ -0,0 +1,76 @@ +#include "RemoveBreakpoint.hpp" + +#include + +#include "src/DebugServer/Gdb/ResponsePackets/OkResponsePacket.hpp" +#include "src/DebugServer/Gdb/ResponsePackets/EmptyResponsePacket.hpp" +#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" + +#include "src/Targets/TargetBreakpoint.hpp" + +#include "src/Services/StringService.hpp" + +#include "src/Logger/Logger.hpp" +#include "src/Exceptions/Exception.hpp" + +namespace DebugServer::Gdb::RiscVGdb::CommandPackets +{ + using Services::TargetControllerService; + + using ResponsePackets::OkResponsePacket; + using ResponsePackets::ErrorResponsePacket; + using ResponsePackets::EmptyResponsePacket; + + using ::Exceptions::Exception; + + RemoveBreakpoint::RemoveBreakpoint(const RawPacket& rawPacket) + : CommandPacket(rawPacket) + { + const auto packetString = std::string{this->data.begin(), this->data.end()}; + + if (packetString.size() < 6) { + throw Exception{"Unexpected RemoveBreakpoint packet size"}; + } + + this->type = (packetString[1] == '0') + ? BreakpointType::SOFTWARE_BREAKPOINT + : (packetString[1] == '1') ? BreakpointType::HARDWARE_BREAKPOINT : BreakpointType::UNKNOWN; + + const auto firstCommaPos = packetString.find_first_of(','); + const auto secondCommaPos = packetString.find_first_of(',', firstCommaPos + 1); + if (firstCommaPos == std::string::npos || secondCommaPos == std::string::npos) { + throw Exception{"Invalid RemoveBreakpoint packet"}; + } + + const auto addressString = packetString.substr(firstCommaPos + 1, secondCommaPos - firstCommaPos - 1); + const auto sizeString = packetString.substr( + secondCommaPos + 1, + packetString.find_first_of(';', secondCommaPos) - secondCommaPos - 1 + ); + + this->address = Services::StringService::toUint32(addressString, 16); + this->size = Services::StringService::toUint32(sizeString, 10); + } + + void RemoveBreakpoint::handle( + DebugSession& debugSession, + const RiscVGdbTargetDescriptor& gdbTargetDescriptor, + const Targets::TargetDescriptor&, + TargetControllerService& targetControllerService + ) { + Logger::info("Handling RemoveBreakpoint packet"); + + try { + debugSession.removeExternalBreakpoint( + gdbTargetDescriptor.systemAddressSpaceDescriptor, + this->address, + targetControllerService + ); + debugSession.connection.writePacket(OkResponsePacket{}); + + } catch (const Exception& exception) { + Logger::error("Failed to remove breakpoint on target - " + exception.getMessage()); + debugSession.connection.writePacket(ErrorResponsePacket{}); + } + } +} diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/RemoveBreakpoint.hpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/RemoveBreakpoint.hpp new file mode 100644 index 00000000..20ad5ef9 --- /dev/null +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/RemoveBreakpoint.hpp @@ -0,0 +1,31 @@ +#pragma once + +#include + +#include "RiscVGdbCommandPacketInterface.hpp" +#include "src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp" +#include "src/DebugServer/Gdb/BreakpointType.hpp" + +#include "src/Targets/TargetMemory.hpp" + +namespace DebugServer::Gdb::RiscVGdb::CommandPackets +{ + class RemoveBreakpoint + : public CommandPackets::RiscVGdbCommandPacketInterface + , private Gdb::CommandPackets::CommandPacket + { + public: + BreakpointType type = BreakpointType::UNKNOWN; + Targets::TargetMemoryAddress address = 0; + Targets::TargetMemorySize size = 0; + + explicit RemoveBreakpoint(const RawPacket& rawPacket); + + void handle( + DebugSession& debugSession, + const RiscVGdbTargetDescriptor& gdbTargetDescriptor, + const Targets::TargetDescriptor& targetDescriptor, + Services::TargetControllerService& targetControllerService + ) override; + }; +} diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp new file mode 100644 index 00000000..b66643e9 --- /dev/null +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.cpp @@ -0,0 +1,103 @@ +#include "SetBreakpoint.hpp" + +#include + +#include "src/DebugServer/Gdb/ResponsePackets/OkResponsePacket.hpp" +#include "src/DebugServer/Gdb/ResponsePackets/EmptyResponsePacket.hpp" +#include "src/DebugServer/Gdb/ResponsePackets/ErrorResponsePacket.hpp" + +#include "src/Targets/TargetBreakpoint.hpp" + +#include "src/Services/StringService.hpp" + +#include "src/Logger/Logger.hpp" +#include "src/Exceptions/Exception.hpp" + +namespace DebugServer::Gdb::RiscVGdb::CommandPackets +{ + using Services::TargetControllerService; + + using ResponsePackets::OkResponsePacket; + using ResponsePackets::ErrorResponsePacket; + using ResponsePackets::EmptyResponsePacket; + + using ::Exceptions::Exception; + + SetBreakpoint::SetBreakpoint(const RawPacket& rawPacket) + : CommandPacket(rawPacket) + { + const auto packetString = std::string{this->data.begin(), this->data.end()}; + + if (packetString.size() < 6) { + throw Exception{"Unexpected SetBreakpoint packet size"}; + } + + // Z0 = SW breakpoint, Z1 = HW breakpoint + this->type = (packetString[1] == '0') + ? BreakpointType::SOFTWARE_BREAKPOINT + : (packetString[1] == '1') ? BreakpointType::HARDWARE_BREAKPOINT : BreakpointType::UNKNOWN; + + const auto firstCommaPos = packetString.find_first_of(','); + const auto secondCommaPos = packetString.find_first_of(',', firstCommaPos + 1); + if (firstCommaPos == std::string::npos || secondCommaPos == std::string::npos) { + throw Exception{"Invalid SetBreakpoint packet"}; + } + + const auto addressString = packetString.substr(firstCommaPos + 1, secondCommaPos - firstCommaPos - 1); + const auto sizeString = packetString.substr( + secondCommaPos + 1, + packetString.find_first_of(';', secondCommaPos) - secondCommaPos - 1 + ); + + this->address = Services::StringService::toUint32(addressString, 16); + this->size = Services::StringService::toUint32(sizeString, 10); + } + + void SetBreakpoint::handle( + DebugSession& debugSession, + const RiscVGdbTargetDescriptor& gdbTargetDescriptor, + const Targets::TargetDescriptor&, + TargetControllerService& targetControllerService + ) { + Logger::info("Handling SetBreakpoint packet"); + + try { + if (this->type == BreakpointType::UNKNOWN) { + Logger::debug( + "Rejecting breakpoint at address " + std::to_string(this->address) + + " - unsupported breakpoint type" + ); + debugSession.connection.writePacket(EmptyResponsePacket{}); + return; + } + + if (this->type == BreakpointType::SOFTWARE_BREAKPOINT && this->size != 2 && this->size != 4) { + throw Exception{"Invalid breakpoint size - expected 2 or 4, got " + std::to_string(this->size)}; + } + + const auto memorySegmentDescriptors = gdbTargetDescriptor.systemAddressSpaceDescriptor.getIntersectingMemorySegmentDescriptors( + Targets::TargetMemoryAddressRange{this->address, this->address + this->size - 1} + ); + + if (memorySegmentDescriptors.size() != 1) { + throw Exception{ + memorySegmentDescriptors.empty() + ? "Invalid breakpoint address - no containing memory segments found for the given address" + : "Invalid breakpoint address - the address range intersects with multiple memory segments" + }; + } + + debugSession.setExternalBreakpoint( + gdbTargetDescriptor.systemAddressSpaceDescriptor, + *(memorySegmentDescriptors.front()), + this->address, + this->size, targetControllerService + ); + debugSession.connection.writePacket(OkResponsePacket{}); + + } catch (const Exception& exception) { + Logger::error("Failed to set breakpoint on target - " + exception.getMessage()); + debugSession.connection.writePacket(ErrorResponsePacket{}); + } + } +} diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.hpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.hpp new file mode 100644 index 00000000..c4e96f17 --- /dev/null +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/SetBreakpoint.hpp @@ -0,0 +1,38 @@ +#pragma once + +#include + +#include "RiscVGdbCommandPacketInterface.hpp" +#include "src/DebugServer/Gdb/CommandPackets/CommandPacket.hpp" +#include "src/DebugServer/Gdb/BreakpointType.hpp" + +#include "src/Targets/TargetMemory.hpp" + +namespace DebugServer::Gdb::RiscVGdb::CommandPackets +{ + class SetBreakpoint + : public CommandPackets::RiscVGdbCommandPacketInterface + , private Gdb::CommandPackets::CommandPacket + { + public: + /** + * The requested breakpoint type. + * + * We don't actually honor this. GDB assumes flash memory cannot be written to outside of a programming + * session, so it refuses to even attempt to insert software breakpoints in flash memory. It always requests + * hardware breakpoints. This is why ignore the requested breakpoint type and just insert whatever type we can. + */ + BreakpointType type = BreakpointType::UNKNOWN; + Targets::TargetMemoryAddress address = 0; + Targets::TargetMemorySize size = 0; + + explicit SetBreakpoint(const RawPacket& rawPacket); + + void handle( + DebugSession& debugSession, + const RiscVGdbTargetDescriptor& gdbTargetDescriptor, + const Targets::TargetDescriptor& targetDescriptor, + Services::TargetControllerService& targetControllerService + ) override; + }; +} diff --git a/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp b/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp index f8a9ad84..955cc5d4 100644 --- a/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp +++ b/src/DebugServer/Gdb/RiscVGdb/RiscVGdbRsp.cpp @@ -9,6 +9,8 @@ #include "CommandPackets/ReadMemory.hpp" #include "CommandPackets/WriteMemory.hpp" #include "CommandPackets/ReadMemoryMap.hpp" +#include "CommandPackets/SetBreakpoint.hpp" +#include "CommandPackets/RemoveBreakpoint.hpp" #include "CommandPackets/FlashErase.hpp" #include "CommandPackets/FlashWrite.hpp" #include "CommandPackets/FlashDone.hpp" @@ -53,6 +55,8 @@ namespace DebugServer::Gdb::RiscVGdb using CommandPackets::ReadMemory; using CommandPackets::WriteMemory; using CommandPackets::ReadMemoryMap; + using CommandPackets::SetBreakpoint; + using CommandPackets::RemoveBreakpoint; using CommandPackets::FlashErase; using CommandPackets::FlashWrite; using CommandPackets::FlashDone; @@ -82,6 +86,14 @@ namespace DebugServer::Gdb::RiscVGdb return std::make_unique(rawPacket, this->gdbTargetDescriptor); } + if (rawPacket[1] == 'Z') { + return std::make_unique(rawPacket); + } + + if (rawPacket[1] == 'z') { + return std::make_unique(rawPacket); + } + if (rawPacket.size() > 1) { const auto rawPacketString = std::string{rawPacket.begin() + 1, rawPacket.end()}; diff --git a/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.cpp index b530e1a1..96dbf05a 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.cpp @@ -242,6 +242,8 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr void EdbgAvr8Interface::deactivate() { if (this->targetAttached) { + this->clearAllBreakpoints(); + if ( this->session.targetConfig.physicalInterface == TargetPhysicalInterface::DEBUG_WIRE && this->session.targetConfig.disableDebugWireOnDeactivate @@ -429,15 +431,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr this->hardwareBreakpointNumbersByAddress.erase(address); } - 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 ) { @@ -1244,6 +1237,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); + } + } + std::unique_ptr EdbgAvr8Interface::getAvrEvent() { auto event = this->edbgInterface->requestAvrEvent(); diff --git a/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.hpp b/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.hpp index ff874e8b..f74a5dad 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.hpp +++ b/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.hpp @@ -201,14 +201,6 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr */ void clearHardwareBreakpoint(Targets::TargetMemoryAddress address) override; - /** - * 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() override; - /** * Reads registers from the target. * @@ -463,6 +455,14 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr */ 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(); + /** * Fetches any queued events belonging to the AVR8 Generic protocol (such as target break events). * diff --git a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp index 812b35d5..79da952c 100644 --- a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp +++ b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp @@ -110,7 +110,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec if (!this->debugModuleDescriptor.triggerDescriptorsByIndex.empty()) { // Clear any left-over triggers from the previous debug session - this->clearAllTriggerBreakpoints(); + this->clearAllTriggers(); } this->initDebugControlStatusRegister(); @@ -122,6 +122,8 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec Logger::debug("Data register count: " + std::to_string(this->debugModuleDescriptor.abstractDataRegisterCount)); Logger::debug("Program buffer size: " + std::to_string(this->debugModuleDescriptor.programBufferSize)); + this->clearProgramBuffer(); + if (this->debugModuleDescriptor.abstractDataRegisterCount > 0) { if (this->debugModuleDescriptor.programBufferSize >= 3) { this->debugModuleDescriptor.memoryAccessStrategies.insert(MemoryAccessStrategy::PROGRAM_BUFFER); @@ -299,15 +301,20 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec void DebugTranslator::insertTriggerBreakpoint(TargetMemoryAddress address) { using TriggerModule::TriggerType; - const auto triggerDescriptorOpt = this->getAvailableTrigger(); + // We may already have a trigger for this address. If so, reuse it. + const auto preexistingTriggerIndexIt = this->triggerIndicesByBreakpointAddress.find(address); + auto triggerDescriptorOpt = preexistingTriggerIndexIt != this->triggerIndicesByBreakpointAddress.end() + ? std::cref(this->debugModuleDescriptor.triggerDescriptorsByIndex.at(preexistingTriggerIndexIt->second)) + : this->getAvailableTrigger(); + if (!triggerDescriptorOpt.has_value()) { throw Exceptions::TargetOperationFailure{"Insufficient resources - no available trigger"}; } const auto& triggerDescriptor = triggerDescriptorOpt->get(); Logger::debug( - "Installing hardware BP at address 0x" + Services::StringService::toHex(address) + " with trigger index " - + std::to_string(triggerDescriptor.index) + "Installing RISC-V trigger for program address 0x" + Services::StringService::toHex(address) + + " with trigger index " + std::to_string(triggerDescriptor.index) ); if (triggerDescriptor.supportedTypes.contains(TriggerType::MATCH_CONTROL)) { @@ -353,7 +360,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec this->allocatedTriggerIndices.erase(triggerDescriptor.index); } - void DebugTranslator::clearAllTriggerBreakpoints() { + void DebugTranslator::clearAllTriggers() { // To ensure that any untracked breakpoints are cleared, we clear all triggers on the target. for (const auto& [triggerIndex, triggerDescriptor] : this->debugModuleDescriptor.triggerDescriptorsByIndex) { this->clearTrigger(triggerDescriptor); @@ -505,6 +512,42 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec throw Exceptions::InternalFatalErrorException{"Unknown selected memory access strategy"}; } + AbstractCommandError DebugTranslator::readAndClearAbstractCommandError() { + const auto commandError = this->readDebugModuleAbstractControlStatusRegister().commandError; + if (commandError != AbstractCommandError::NONE) { + this->clearAbstractCommandError(); + } + + return commandError; + } + + void DebugTranslator::clearProgramBuffer() { + if (this->debugModuleDescriptor.programBufferSize < 1) { + return; + } + + this->writeProgramBuffer( + std::vector(this->debugModuleDescriptor.programBufferSize, Opcodes::Ebreak) + ); + } + + void DebugTranslator::executeFenceProgram() { + static constexpr auto programOpcodes = std::to_array({ + Opcodes::FenceI, + Opcodes::Fence, + Opcodes::Ebreak, + }); + + if (programOpcodes.size() > this->debugModuleDescriptor.programBufferSize) { + throw Exceptions::TargetOperationFailure{ + "Cannot execute fence program via RISC-V debug module program buffer - insufficient program buffer size" + }; + } + + this->writeProgramBuffer(programOpcodes); + this->readCpuRegister(CpuRegisterNumber::GPR_X8, {.postExecute = true}); + } + std::vector DebugTranslator::discoverHartIndices() { auto hartIndices = std::vector{}; diff --git a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp index 594c301c..43697ef8 100644 --- a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp +++ b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp @@ -68,7 +68,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec std::uint16_t getTriggerCount() const; void insertTriggerBreakpoint(Targets::TargetMemoryAddress address); void clearTriggerBreakpoint(Targets::TargetMemoryAddress address); - void clearAllTriggerBreakpoints(); + void clearAllTriggers(); Targets::TargetRegisterDescriptorAndValuePairs readCpuRegisters( const Targets::TargetRegisterDescriptors& descriptors @@ -89,6 +89,21 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec Targets::TargetMemoryBufferSpan buffer ); + DebugModule::AbstractCommandError readAndClearAbstractCommandError(); + + /** + * Fills the program buffer with ebreak instructions. + */ + void clearProgramBuffer(); + + /** + * This isn't used anywhere ATM. I don't think it's needed, as Bloom only supports RISC-V targets that have a + * single hart. + * + * TODO: Consider removing + */ + void executeFenceProgram(); + private: static constexpr auto DEBUG_MODULE_RESPONSE_DELAY = std::chrono::microseconds{10}; static constexpr auto WORD_BYTE_SIZE = ::Targets::TargetMemorySize{4}; diff --git a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR8/Avr8DebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR8/Avr8DebugInterface.hpp index 11328c31..3b7825a3 100644 --- a/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR8/Avr8DebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/Microchip/AVR8/Avr8DebugInterface.hpp @@ -100,7 +100,7 @@ namespace DebugToolDrivers::TargetInterfaces::Microchip::Avr8 virtual void clearSoftwareBreakpoint(Targets::TargetMemoryAddress address) = 0; virtual void setHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; virtual void clearHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - virtual void clearAllBreakpoints() = 0; + virtual Targets::TargetMemoryAddress getProgramCounter() = 0; virtual void setProgramCounter(Targets::TargetMemoryAddress programCounter) = 0; virtual Targets::TargetRegisterDescriptorAndValuePairs readRegisters( diff --git a/src/DebugToolDrivers/TargetInterfaces/RiscV/RiscVDebugInterface.hpp b/src/DebugToolDrivers/TargetInterfaces/RiscV/RiscVDebugInterface.hpp index 899bc7ea..bde8a89f 100644 --- a/src/DebugToolDrivers/TargetInterfaces/RiscV/RiscVDebugInterface.hpp +++ b/src/DebugToolDrivers/TargetInterfaces/RiscV/RiscVDebugInterface.hpp @@ -11,6 +11,7 @@ #include "src/Targets/TargetState.hpp" #include "src/Targets/TargetRegisterDescriptor.hpp" #include "src/Targets/TargetMemory.hpp" +#include "src/Targets/TargetBreakpoint.hpp" namespace DebugToolDrivers::TargetInterfaces::RiscV { @@ -29,13 +30,9 @@ namespace DebugToolDrivers::TargetInterfaces::RiscV virtual void step() = 0; virtual void reset() = 0; - virtual void setSoftwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - virtual void clearSoftwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - - virtual std::uint16_t getHardwareBreakpointCount() = 0; - virtual void setHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - virtual void clearHardwareBreakpoint(Targets::TargetMemoryAddress address) = 0; - virtual void clearAllHardwareBreakpoints() = 0; + virtual Targets::BreakpointResources getBreakpointResources() = 0; + virtual void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) = 0; + virtual void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) = 0; virtual Targets::TargetRegisterDescriptorAndValuePairs readCpuRegisters( const Targets::TargetRegisterDescriptors& descriptors @@ -59,5 +56,8 @@ namespace DebugToolDrivers::TargetInterfaces::RiscV const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor ) = 0; + + virtual void enableProgrammingMode() = 0; + virtual void disableProgrammingMode() = 0; }; } diff --git a/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp b/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp index dd225afb..4bd6c4a9 100644 --- a/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp +++ b/src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp @@ -1,5 +1,7 @@ #include "WchLinkDebugInterface.hpp" +#include + #include "Protocols/WchLink/Commands/Control/AttachTarget.hpp" #include "Protocols/WchLink/Commands/Control/DetachTarget.hpp" #include "Protocols/WchLink/Commands/Control/PostAttach.hpp" @@ -8,7 +10,12 @@ #include "Protocols/WchLink/FlashProgramOpcodes.hpp" +#include "src/Targets/RiscV/Opcodes/Opcode.hpp" + #include "src/Services/StringService.hpp" + +#include "src/Exceptions/InternalFatalErrorException.hpp" +#include "src/TargetController/Exceptions/TargetOperationFailure.hpp" #include "src/Targets/TargetDescription/Exceptions/InvalidTargetDescriptionDataException.hpp" #include "src/Logger/Logger.hpp" @@ -24,6 +31,7 @@ namespace DebugToolDrivers::Wch using ::Targets::TargetStackPointer; using ::Targets::TargetAddressSpaceDescriptor; using ::Targets::TargetMemorySegmentDescriptor; + using ::Targets::TargetProgramBreakpoint; using ::Targets::TargetMemorySegmentType; using ::Targets::TargetRegisterDescriptors; using ::Targets::TargetRegisterDescriptorAndValuePairs; @@ -113,6 +121,7 @@ namespace DebugToolDrivers::Wch } void WchLinkDebugInterface::deactivate() { + this->riscVTranslator.clearAllTriggers(); this->riscVTranslator.deactivate(); const auto response = this->wchLinkInterface.sendCommandAndWaitForResponse(Commands::Control::DetachTarget{}); @@ -145,28 +154,29 @@ namespace DebugToolDrivers::Wch this->riscVTranslator.reset(); } - void WchLinkDebugInterface::setSoftwareBreakpoint(Targets::TargetMemoryAddress address) { - throw Exceptions::Exception{"SW breakpoints not supported"}; + Targets::BreakpointResources WchLinkDebugInterface::getBreakpointResources() { + return { + .hardwareBreakpoints = this->riscVTranslator.getTriggerCount(), + .softwareBreakpoints = 0xFFFFFFFF, // TODO: Use the program memory size to determine the limit. + }; } - void WchLinkDebugInterface::clearSoftwareBreakpoint(Targets::TargetMemoryAddress address) { - throw Exceptions::Exception{"SW breakpoints not supported"}; + void WchLinkDebugInterface::setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { + this->riscVTranslator.insertTriggerBreakpoint(breakpoint.address); + + } else { + this->setSoftwareBreakpoint(breakpoint); + } } - std::uint16_t WchLinkDebugInterface::getHardwareBreakpointCount() { - return this->riscVTranslator.getTriggerCount(); - } + void WchLinkDebugInterface::removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { + this->riscVTranslator.clearTriggerBreakpoint(breakpoint.address); - void WchLinkDebugInterface::setHardwareBreakpoint(Targets::TargetMemoryAddress address) { - this->riscVTranslator.insertTriggerBreakpoint(address); - } - - void WchLinkDebugInterface::clearHardwareBreakpoint(Targets::TargetMemoryAddress address) { - this->riscVTranslator.clearTriggerBreakpoint(address); - } - - void WchLinkDebugInterface::clearAllHardwareBreakpoints() { - this->riscVTranslator.clearAllTriggerBreakpoints(); + } else { + this->clearSoftwareBreakpoint(breakpoint); + } } Targets::TargetRegisterDescriptorAndValuePairs WchLinkDebugInterface::readCpuRegisters( @@ -203,60 +213,121 @@ namespace DebugToolDrivers::Wch ) { if (memorySegmentDescriptor.type == TargetMemorySegmentType::FLASH) { /* - * WCH-Link tools cannot write to flash memory via the target's debug module. + * WCH-Link tools cannot write to flash memory via the target's debug module. They do, however, offer a + * set of dedicated commands for this. We invoke them here. * - * They do, however, offer a set of dedicated commands for writing to flash memory. We invoke them here. + * There are two commands we can choose from: * - * See WchLinkDebugInterface::writeFlashMemory() below, for more. + * - Partial block write + * Writes any number of bytes to flash, but limited to a maximum of 64 bytes per write. Larger writes + * must be split into multiple writes. + * - Full block write + * Writes an entire block to flash, where the block size is target-specific (resides in the target's + * TDF). Requires alignment to the block size. Requires reattaching to the target at the end of the + * programming session. + * + * The full block write is much faster for writing large buffers (KiBs), such as when we're programming + * the target. But the partial block write is faster and more suitable for writing buffers that are + * smaller than 64 bytes, such as when we're inserting software breakpoints. */ const auto bufferSize = static_cast(buffer.size()); - const auto alignmentSize = bufferSize > WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE - ? this->programmingBlockSize - : 1; - if (alignmentSize > 1) { - const auto alignedStartAddress = (startAddress / alignmentSize) * alignmentSize; - const auto alignedBufferSize = static_cast(std::ceil( - static_cast(bufferSize) / static_cast(alignmentSize) - ) * alignmentSize); + if (bufferSize <= WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE) { + using namespace ::DebugToolDrivers::Protocols::RiscVDebugSpec; + /* + * WCH-Link tools seem to make use of the target's program buffer to service the partial block write + * command. + * + * This sometimes leads to exceptions occurring on the target, when the program buffer contains certain + * instructions before the partial block write command is serviced. This is why we clear the program + * buffer before invoking the partial block write command. + */ + this->riscVTranslator.clearProgramBuffer(); + this->wchLinkInterface.writeFlashPartialBlock(startAddress, buffer); - if (alignedStartAddress != startAddress || alignedBufferSize != bufferSize) { - auto alignedBuffer = (alignedStartAddress < startAddress) - ? this->readMemory( - addressSpaceDescriptor, - memorySegmentDescriptor, - alignedStartAddress, - (startAddress - alignedStartAddress), - {} - ) - : TargetMemoryBuffer{}; - - alignedBuffer.resize(alignedBufferSize); - - std::copy( - buffer.begin(), - buffer.end(), - alignedBuffer.begin() + (startAddress - alignedStartAddress) - ); - - const auto dataBack = this->readMemory( - addressSpaceDescriptor, - memorySegmentDescriptor, - startAddress + bufferSize, - alignedBufferSize - bufferSize - (startAddress - alignedStartAddress), - {} - ); - std::copy( - dataBack.begin(), - dataBack.end(), - alignedBuffer.begin() + (startAddress - alignedStartAddress) + bufferSize - ); - - return this->writeFlashMemory(alignedStartAddress, alignedBuffer); + const auto commandError = this->riscVTranslator.readAndClearAbstractCommandError(); + if (commandError != DebugModule::AbstractCommandError::NONE) { + throw Exceptions::Exception{ + "Partial block write failed - abstract command error: 0x" + + Services::StringService::toHex(commandError) + }; } + + return; } - return this->writeFlashMemory(startAddress, buffer); + const auto alignmentSize = this->programmingBlockSize; + const auto alignedStartAddress = (startAddress / alignmentSize) * alignmentSize; + const auto alignedBufferSize = static_cast(std::ceil( + static_cast(bufferSize) / static_cast(alignmentSize) + ) * alignmentSize); + + if (alignedStartAddress != startAddress || alignedBufferSize != bufferSize) { + if ( + !memorySegmentDescriptor.addressRange.contains( + TargetMemoryAddressRange{ + alignedStartAddress, + alignedStartAddress + alignedBufferSize - 1 + } + ) + ) { + /* + * TODO: The aligned address range exceeds the bounds of the memory segment. I'm not sure what to + * do here. We could just ignore it...I don't think it will cause much of an issue, for now. + * Review (after v2.0.0, maybe?). + */ + } + + auto alignedBuffer = (alignedStartAddress < startAddress) + ? this->readMemory( + addressSpaceDescriptor, + memorySegmentDescriptor, + alignedStartAddress, + (startAddress - alignedStartAddress), + {} + ) + : TargetMemoryBuffer{}; + + alignedBuffer.resize(alignedBufferSize); + + std::copy( + buffer.begin(), + buffer.end(), + alignedBuffer.begin() + (startAddress - alignedStartAddress) + ); + + const auto dataBack = this->readMemory( + addressSpaceDescriptor, + memorySegmentDescriptor, + startAddress + bufferSize, + alignedBufferSize - bufferSize - (startAddress - alignedStartAddress), + {} + ); + std::copy( + dataBack.begin(), + dataBack.end(), + alignedBuffer.begin() + (startAddress - alignedStartAddress) + bufferSize + ); + + return this->writeMemory( + addressSpaceDescriptor, + memorySegmentDescriptor, + alignedStartAddress, + alignedBuffer + ); + } + + this->wchLinkInterface.writeFlashFullBlocks( + startAddress, + buffer, + this->programmingBlockSize, + this->flashProgramOpcodes + ); + + this->deactivate(); + this->wchLinkInterface.sendCommandAndWaitForResponse(Commands::Control::GetDeviceInfo{}); + this->activate(); + return; } this->riscVTranslator.writeMemory( @@ -278,36 +349,96 @@ namespace DebugToolDrivers::Wch throw Exception{"Erasing non-flash memory not supported in WchLinkDebugInterface"}; } - void WchLinkDebugInterface::writeFlashMemory(TargetMemoryAddress startAddress, TargetMemoryBufferSpan buffer) { - /* - * There are two commands we can choose from when writing to flash memory: - * - * - Partial block write - * Writes any number of bytes to flash, but limited to a maximum of 64 bytes per write. Larger writes - * must be split into multiple writes. - * - Full block write - * Writes an entire block to flash, where the block size is target-specific (resides in the target's - * TDF). Requires alignment to the block size. Requires reattaching to the target at the end of the - * programming session. - * - * The full block write is much faster for writing large buffers (KiBs), such as when we're programming the - * target. But the partial block write is faster and more suitable for writing buffers that are smaller than - * 64 bytes, such as when we're inserting software breakpoints. - */ - if (buffer.size() <= WchLinkInterface::MAX_PARTIAL_BLOCK_WRITE_SIZE) { - return this->wchLinkInterface.writeFlashPartialBlock(startAddress, buffer); + void WchLinkDebugInterface::enableProgrammingMode() { + // Nothing to do here + } + + void WchLinkDebugInterface::disableProgrammingMode() { + this->softwareBreakpointRegistry.clear(); + } + + void WchLinkDebugInterface::setSoftwareBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.size != 2 && breakpoint.size != 4) { + throw Exception{"Invalid software breakpoint size (" + std::to_string(breakpoint.size) + ")"}; } - this->wchLinkInterface.writeFlashFullBlocks( - startAddress, - buffer, - this->programmingBlockSize, - this->flashProgramOpcodes + const auto originalData = this->readMemory( + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size, + {} ); - this->deactivate(); - this->wchLinkInterface.sendCommandAndWaitForResponse(Commands::Control::GetDeviceInfo{}); - this->activate(); + 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 ebreakBytes = std::to_array({ + static_cast(::Targets::RiscV::Opcodes::Ebreak), + static_cast(::Targets::RiscV::Opcodes::Ebreak >> 8), + static_cast(::Targets::RiscV::Opcodes::Ebreak >> 16), + static_cast(::Targets::RiscV::Opcodes::Ebreak >> 24) + }); + + static constexpr auto compressedEbreakBytes = std::to_array({ + static_cast(::Targets::RiscV::Opcodes::EbreakCompressed), + static_cast(::Targets::RiscV::Opcodes::EbreakCompressed >> 8) + }); + + this->writeMemory( + softwareBreakpoint.addressSpaceDescriptor, + softwareBreakpoint.memorySegmentDescriptor, + softwareBreakpoint.address, + softwareBreakpoint.size == 2 + ? TargetMemoryBufferSpan{compressedEbreakBytes} + : TargetMemoryBufferSpan{ebreakBytes} + ); + + this->softwareBreakpointRegistry.insert(softwareBreakpoint); + } + + void WchLinkDebugInterface::clearSoftwareBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.size != 2 && breakpoint.size != 4) { + 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) + } + ); + + this->softwareBreakpointRegistry.remove(softwareBreakpoint); } void WchLinkDebugInterface::eraseFlashMemory() { diff --git a/src/DebugToolDrivers/WCH/WchLinkDebugInterface.hpp b/src/DebugToolDrivers/WCH/WchLinkDebugInterface.hpp index 548a4427..d8aa23f4 100644 --- a/src/DebugToolDrivers/WCH/WchLinkDebugInterface.hpp +++ b/src/DebugToolDrivers/WCH/WchLinkDebugInterface.hpp @@ -12,7 +12,10 @@ #include "src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp" #include "src/Targets/TargetMemory.hpp" +#include "src/Targets/ProgramBreakpointRegistry.hpp" + #include "src/Targets/RiscV/Wch/TargetDescriptionFile.hpp" +#include "src/Targets/RiscV/ProgramBreakpoint.hpp" namespace DebugToolDrivers::Wch { @@ -45,13 +48,9 @@ namespace DebugToolDrivers::Wch void step() override; void reset() override; - void setSoftwareBreakpoint(Targets::TargetMemoryAddress address) override; - void clearSoftwareBreakpoint(Targets::TargetMemoryAddress address) override; - - std::uint16_t getHardwareBreakpointCount() override; - void setHardwareBreakpoint(Targets::TargetMemoryAddress address) override; - void clearHardwareBreakpoint(Targets::TargetMemoryAddress address) override; - void clearAllHardwareBreakpoints() override; + Targets::BreakpointResources getBreakpointResources() override; + void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; + void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) override; Targets::TargetRegisterDescriptorAndValuePairs readCpuRegisters( const Targets::TargetRegisterDescriptors& descriptors @@ -76,6 +75,9 @@ namespace DebugToolDrivers::Wch const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor ) override; + void enableProgrammingMode() override; + void disableProgrammingMode() override; + private: const WchLinkToolConfig& toolConfig; const Targets::RiscV::RiscVTargetConfig& targetConfig; @@ -94,12 +96,15 @@ namespace DebugToolDrivers::Wch std::optional cachedVariantId; std::optional cachedTargetId; + Targets::ProgramBreakpointRegistryGeneric softwareBreakpointRegistry; + std::span flashProgramOpcodes; Targets::TargetMemorySize programmingBlockSize; - void writeFlashMemory(Targets::TargetMemoryAddress startAddress, Targets::TargetMemoryBufferSpan buffer); - void eraseFlashMemory(); + void setSoftwareBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint); + void clearSoftwareBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint); + void eraseFlashMemory(); static std::span getFlashProgramOpcodes(const std::string& key); }; } diff --git a/src/Services/TargetControllerService.cpp b/src/Services/TargetControllerService.cpp index 278f1dbf..4c07300f 100644 --- a/src/Services/TargetControllerService.cpp +++ b/src/Services/TargetControllerService.cpp @@ -14,8 +14,8 @@ #include "src/TargetController/Commands/WriteTargetMemory.hpp" #include "src/TargetController/Commands/EraseTargetMemory.hpp" #include "src/TargetController/Commands/StepTargetExecution.hpp" -#include "src/TargetController/Commands/SetBreakpoint.hpp" -#include "src/TargetController/Commands/RemoveBreakpoint.hpp" +#include "src/TargetController/Commands/SetProgramBreakpointAnyType.hpp" +#include "src/TargetController/Commands/RemoveProgramBreakpoint.hpp" #include "src/TargetController/Commands/SetTargetProgramCounter.hpp" #include "src/TargetController/Commands/SetTargetStackPointer.hpp" #include "src/TargetController/Commands/GetTargetGpioPadStates.hpp" @@ -43,8 +43,8 @@ namespace Services using TargetController::Commands::WriteTargetMemory; using TargetController::Commands::EraseTargetMemory; using TargetController::Commands::StepTargetExecution; - using TargetController::Commands::SetBreakpoint; - using TargetController::Commands::RemoveBreakpoint; + using TargetController::Commands::SetProgramBreakpointAnyType; + using TargetController::Commands::RemoveProgramBreakpoint; using TargetController::Commands::SetTargetProgramCounter; using TargetController::Commands::SetTargetStackPointer; using TargetController::Commands::GetTargetGpioPadStates; @@ -71,8 +71,6 @@ namespace Services using Targets::TargetMemoryBufferSpan; using Targets::TargetStackPointer; - using Targets::TargetBreakpoint; - using Targets::TargetPinoutDescriptor; using Targets::TargetPinDescriptor; using Targets::TargetGpioPadState; @@ -236,20 +234,27 @@ namespace Services ); } - Targets::TargetBreakpoint TargetControllerService::setBreakpoint( + Targets::TargetProgramBreakpoint TargetControllerService::setProgramBreakpointAnyType( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, Targets::TargetMemoryAddress address, - Targets::TargetBreakpoint::Type preferredType + Targets::TargetMemorySize size ) const { return this->commandManager.sendCommandAndWaitForResponse( - std::make_unique(address, preferredType), + std::make_unique( + addressSpaceDescriptor, + memorySegmentDescriptor, + address, + size + ), this->defaultTimeout, this->activeAtomicSessionId )->breakpoint; } - void TargetControllerService::removeBreakpoint(TargetBreakpoint breakpoint) const { + void TargetControllerService::removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) const { this->commandManager.sendCommandAndWaitForResponse( - std::make_unique(breakpoint), + std::make_unique(breakpoint), this->defaultTimeout, this->activeAtomicSessionId ); diff --git a/src/Services/TargetControllerService.hpp b/src/Services/TargetControllerService.hpp index 64dfdc3c..008f8e7a 100644 --- a/src/Services/TargetControllerService.hpp +++ b/src/Services/TargetControllerService.hpp @@ -170,25 +170,29 @@ namespace Services ) const; /** - * Requests the TargetController to set a breakpoint on the target. + * Requests the TargetController to set a program breakpoint of any type (hardware/software) on the target. * + * @param addressSpaceDescriptor + * @param memorySegmentDescriptor * @param address - * @param preferredType + * @param size * * @return * The installed breakpoint. */ - [[nodiscard]] Targets::TargetBreakpoint setBreakpoint( + [[nodiscard]] Targets::TargetProgramBreakpoint setProgramBreakpointAnyType( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, Targets::TargetMemoryAddress address, - Targets::TargetBreakpoint::Type preferredType = Targets::TargetBreakpoint::Type::HARDWARE + Targets::TargetMemorySize size ) const; /** - * Requests the TargetController to remove a breakpoint from the target. + * Requests the TargetController to remove a program breakpoint from the target. * * @param breakpoint */ - void removeBreakpoint(Targets::TargetBreakpoint breakpoint) const; + void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) const; /** * Retrieves the current program counter value from the target. diff --git a/src/TargetController/Commands/CommandTypes.hpp b/src/TargetController/Commands/CommandTypes.hpp index 3a8a6bfc..99d2bf0a 100644 --- a/src/TargetController/Commands/CommandTypes.hpp +++ b/src/TargetController/Commands/CommandTypes.hpp @@ -21,8 +21,8 @@ namespace TargetController::Commands ERASE_TARGET_MEMORY, GET_TARGET_STATE, STEP_TARGET_EXECUTION, - SET_BREAKPOINT, - REMOVE_BREAKPOINT, + SET_BREAKPOINT_ANY_TYPE, + REMOVE_PROGRAM_BREAKPOINT, SET_TARGET_PROGRAM_COUNTER, SET_TARGET_STACK_POINTER, GET_TARGET_GPIO_PAD_STATES, diff --git a/src/TargetController/Commands/RemoveBreakpoint.hpp b/src/TargetController/Commands/RemoveProgramBreakpoint.hpp similarity index 56% rename from src/TargetController/Commands/RemoveBreakpoint.hpp rename to src/TargetController/Commands/RemoveProgramBreakpoint.hpp index 2273ecfc..f5fd4a32 100644 --- a/src/TargetController/Commands/RemoveBreakpoint.hpp +++ b/src/TargetController/Commands/RemoveProgramBreakpoint.hpp @@ -6,21 +6,20 @@ namespace TargetController::Commands { - class RemoveBreakpoint: public Command + class RemoveProgramBreakpoint: public Command { public: - static constexpr CommandType type = CommandType::REMOVE_BREAKPOINT; - static const inline std::string name = "RemoveBreakpoint"; + static constexpr CommandType type = CommandType::REMOVE_PROGRAM_BREAKPOINT; + static const inline std::string name = "RemoveProgramBreakpoint"; - Targets::TargetBreakpoint breakpoint; + Targets::TargetProgramBreakpoint breakpoint; - RemoveBreakpoint() = default; - explicit RemoveBreakpoint(const Targets::TargetBreakpoint& breakpoint) + explicit RemoveProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) : breakpoint(breakpoint) {}; [[nodiscard]] CommandType getType() const override { - return RemoveBreakpoint::type; + return RemoveProgramBreakpoint::type; } [[nodiscard]] bool requiresStoppedTargetState() const override { diff --git a/src/TargetController/Commands/SetBreakpoint.hpp b/src/TargetController/Commands/SetBreakpoint.hpp deleted file mode 100644 index e4c8d863..00000000 --- a/src/TargetController/Commands/SetBreakpoint.hpp +++ /dev/null @@ -1,47 +0,0 @@ -#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"; - - /** - * Byte address in program memory. - */ - Targets::TargetMemoryAddress address; - - /** - * 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 { - return SetBreakpoint::type; - } - - [[nodiscard]] bool requiresStoppedTargetState() const override { - return true; - } - }; -} diff --git a/src/TargetController/Commands/SetProgramBreakpointAnyType.hpp b/src/TargetController/Commands/SetProgramBreakpointAnyType.hpp new file mode 100644 index 00000000..52fd7b27 --- /dev/null +++ b/src/TargetController/Commands/SetProgramBreakpointAnyType.hpp @@ -0,0 +1,45 @@ +#pragma once + +#include "Command.hpp" +#include "src/TargetController/Responses/ProgramBreakpoint.hpp" + +#include "src/Targets/TargetAddressSpaceDescriptor.hpp" +#include "src/Targets/TargetMemorySegmentDescriptor.hpp" +#include "src/Targets/TargetMemory.hpp" + +namespace TargetController::Commands +{ + class SetProgramBreakpointAnyType: public Command + { + public: + using SuccessResponseType = Responses::ProgramBreakpoint; + + static constexpr CommandType type = CommandType::SET_BREAKPOINT_ANY_TYPE; + static const inline std::string name = "SetProgramBreakpointAnyType"; + + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor; + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor; + Targets::TargetMemoryAddress address; + Targets::TargetMemorySize size; + + SetProgramBreakpointAnyType( + const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor, + const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor, + Targets::TargetMemoryAddress address, + Targets::TargetMemorySize size + ) + : addressSpaceDescriptor(addressSpaceDescriptor) + , memorySegmentDescriptor(memorySegmentDescriptor) + , address(address) + , size(size) + {}; + + [[nodiscard]] CommandType getType() const override { + return SetProgramBreakpointAnyType::type; + } + + [[nodiscard]] bool requiresStoppedTargetState() const override { + return true; + } + }; +} diff --git a/src/TargetController/Responses/Breakpoint.hpp b/src/TargetController/Responses/Breakpoint.hpp deleted file mode 100644 index 6ab52166..00000000 --- a/src/TargetController/Responses/Breakpoint.hpp +++ /dev/null @@ -1,24 +0,0 @@ -#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/ProgramBreakpoint.hpp b/src/TargetController/Responses/ProgramBreakpoint.hpp new file mode 100644 index 00000000..5f567d1b --- /dev/null +++ b/src/TargetController/Responses/ProgramBreakpoint.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include "Response.hpp" + +#include "src/Targets/TargetBreakpoint.hpp" + +namespace TargetController::Responses +{ + class ProgramBreakpoint: public Response + { + public: + static constexpr ResponseType type = ResponseType::PROGRAM_BREAKPOINT; + + Targets::TargetProgramBreakpoint breakpoint; + + explicit ProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint) + : breakpoint(breakpoint) + {} + + [[nodiscard]] ResponseType getType() const override { + return ProgramBreakpoint::type; + } + }; +} diff --git a/src/TargetController/Responses/ResponseTypes.hpp b/src/TargetController/Responses/ResponseTypes.hpp index 06ec44df..28645649 100644 --- a/src/TargetController/Responses/ResponseTypes.hpp +++ b/src/TargetController/Responses/ResponseTypes.hpp @@ -16,6 +16,6 @@ namespace TargetController::Responses TARGET_GPIO_PAD_STATES, TARGET_STACK_POINTER, TARGET_PROGRAM_COUNTER, - BREAKPOINT, + PROGRAM_BREAKPOINT, }; } diff --git a/src/TargetController/TargetControllerComponent.cpp b/src/TargetController/TargetControllerComponent.cpp index 1eeac69c..60090866 100644 --- a/src/TargetController/TargetControllerComponent.cpp +++ b/src/TargetController/TargetControllerComponent.cpp @@ -41,8 +41,8 @@ namespace TargetController using Commands::WriteTargetMemory; using Commands::EraseTargetMemory; using Commands::StepTargetExecution; - using Commands::SetBreakpoint; - using Commands::RemoveBreakpoint; + using Commands::SetProgramBreakpointAnyType; + using Commands::RemoveProgramBreakpoint; using Commands::SetTargetProgramCounter; using Commands::SetTargetStackPointer; using Commands::GetTargetGpioPadStates; @@ -59,7 +59,7 @@ namespace TargetController using Responses::TargetGpioPadStates; using Responses::TargetStackPointer; using Responses::TargetProgramCounter; - using Responses::Breakpoint; + using Responses::ProgramBreakpoint; TargetControllerComponent::TargetControllerComponent( const ProjectConfig& projectConfig, @@ -218,12 +218,16 @@ namespace TargetController std::bind(&TargetControllerComponent::handleStepTargetExecution, this, std::placeholders::_1) ); - this->registerCommandHandler( - std::bind(&TargetControllerComponent::handleSetBreakpoint, this, std::placeholders::_1) + this->registerCommandHandler( + std::bind( + &TargetControllerComponent::handleSetProgramBreakpointBreakpointAnyType, + this, + std::placeholders::_1 + ) ); - this->registerCommandHandler( - std::bind(&TargetControllerComponent::handleRemoveBreakpoint, this, std::placeholders::_1) + this->registerCommandHandler( + std::bind(&TargetControllerComponent::handleRemoveProgramBreakpoint, this, std::placeholders::_1) ); this->registerCommandHandler( @@ -299,6 +303,17 @@ namespace TargetController // Reject any commands still waiting in the queue this->processQueuedCommands(); + if (this->target != nullptr) { + // Cleanup before deactivating the target + try { + this->stopTarget(); + this->clearAllBreakpoints(); + + } catch (const Exception& exception) { + Logger::error("Target pre-deactivation cleanup failed: " + exception.getMessage()); + } + } + this->releaseHardware(); } catch (const std::exception& exception) { @@ -540,24 +555,15 @@ namespace TargetController ); this->refreshExecutionState(); - if (!this->environmentConfig.targetConfig.hardwareBreakpoints) { - Logger::warning("Hardware breakpoints have been disabled"); + 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 { - const auto& breakpointResources = this->targetDescriptor->breakpointResources; - if (breakpointResources.maximumHardwareBreakpoints.has_value()) { - Logger::info( - "Available hardware breakpoints: " + std::to_string( - *(breakpointResources.maximumHardwareBreakpoints) - ) - ); - } - - if (breakpointResources.reservedHardwareBreakpoints > 0) { - Logger::info( - "Reserved hardware breakpoints: " + std::to_string(breakpointResources.reservedHardwareBreakpoints) - ); - } + Logger::warning("Hardware breakpoints have been disabled"); } } @@ -571,10 +577,6 @@ namespace TargetController if (debugTool != nullptr && debugTool->isInitialised()) { if (target != nullptr) { - /* - * We call deactivate() without checking if the target is activated. This will address any cases - * where a target is only partially activated (where the call to activate() failed). - */ Logger::info("Deactivating target"); target->deactivate(); } @@ -795,40 +797,114 @@ namespace TargetController this->target->eraseMemory(addressSpaceDescriptor, memorySegmentDescriptor); } - void TargetControllerComponent::setBreakpoint(const TargetBreakpoint& breakpoint) { - using Services::StringService; - - if (breakpoint.type == TargetBreakpoint::Type::HARDWARE) { - Logger::debug( - "Installing hardware breakpoint at byte address 0x" + StringService::toHex(breakpoint.address) - ); - - this->target->setHardwareBreakpoint(breakpoint.address); - this->hardwareBreakpointsByAddress.emplace(breakpoint.address, breakpoint); - return; - } - - Logger::debug("Installing software breakpoint at byte address 0x" + StringService::toHex(breakpoint.address)); - - this->target->setSoftwareBreakpoint(breakpoint.address); - this->softwareBreakpointsByAddress.emplace(breakpoint.address, breakpoint); + std::uint32_t TargetControllerComponent::availableHardwareBreakpoints() { + const auto& targetBreakpointResources = this->targetDescriptor->breakpointResources; + return static_cast( + targetBreakpointResources.hardwareBreakpoints - targetBreakpointResources.reservedHardwareBreakpoints + - this->hardwareBreakpointRegistry.size() + ); } - void TargetControllerComponent::removeBreakpoint(const TargetBreakpoint& breakpoint) { + void TargetControllerComponent::setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { using Services::StringService; - if (breakpoint.type == Targets::TargetBreakpoint::Type::HARDWARE) { - Logger::debug("Removing hardware breakpoint at byte address 0x" + StringService::toHex(breakpoint.address)); + auto& registry = breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE + ? this->hardwareBreakpointRegistry + : this->softwareBreakpointRegistry; - this->target->removeHardwareBreakpoint(breakpoint.address); - this->hardwareBreakpointsByAddress.erase(breakpoint.address); + Logger::debug("Inserting breakpoint at byte address 0x" + StringService::toHex(breakpoint.address)); + + if (registry.contains(breakpoint)) { + Logger::debug("Breakpoint already in registry - ignoring insertion request"); return; } - Logger::debug("Removing software breakpoint at byte address 0x" + StringService::toHex(breakpoint.address)); + this->target->setProgramBreakpoint(breakpoint); + registry.insert(breakpoint); - this->target->removeSoftwareBreakpoint(breakpoint.address); - this->softwareBreakpointsByAddress.erase(breakpoint.address); + if ( + breakpoint.type == TargetProgramBreakpoint::Type::SOFTWARE + && this->environmentConfig.targetConfig.programMemoryCache + && this->target->isProgramMemory( + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size + ) + ) { + auto& cache = this->getProgramMemoryCache(breakpoint.memorySegmentDescriptor); + if (cache.contains(breakpoint.address, breakpoint.size)) { + // Update program memory cache + cache.insert( + breakpoint.address, + this->target->readMemory( + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size, + {} + ) + ); + } + } + } + + void TargetControllerComponent::removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + using Services::StringService; + + auto& registry = breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE + ? this->hardwareBreakpointRegistry + : this->softwareBreakpointRegistry; + + Logger::debug("Removing breakpoint at byte address 0x" + StringService::toHex(breakpoint.address)); + + if (!registry.contains(breakpoint)) { + Logger::debug("Breakpoint not found in registry - ignoring removal request"); + return; + } + + this->target->removeProgramBreakpoint(breakpoint); + registry.remove(breakpoint); + + if ( + breakpoint.type == TargetProgramBreakpoint::Type::SOFTWARE + && this->environmentConfig.targetConfig.programMemoryCache + && this->target->isProgramMemory( + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size + ) + ) { + auto& cache = this->getProgramMemoryCache(breakpoint.memorySegmentDescriptor); + if (cache.contains(breakpoint.address, breakpoint.size)) { + // Update program memory cache + cache.insert( + breakpoint.address, + this->target->readMemory( + breakpoint.addressSpaceDescriptor, + breakpoint.memorySegmentDescriptor, + breakpoint.address, + breakpoint.size, + {} + ) + ); + } + } + } + + void TargetControllerComponent::clearAllBreakpoints() { + for (const auto& [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->removeProgramBreakpoint(breakpoint); + } + } + + for (const auto& [addressSpaceId, breakpointsByAddress] : this->hardwareBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->removeProgramBreakpoint(breakpoint); + } + } } void TargetControllerComponent::enableProgrammingMode() { @@ -849,12 +925,16 @@ namespace TargetController Logger::info("Restoring breakpoints"); this->target->stop(); - for (const auto& [address, breakpoint] : this->softwareBreakpointsByAddress) { - this->target->setSoftwareBreakpoint(address); + for (const auto& [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->target->setProgramBreakpoint(breakpoint); + } } - for (const auto& [address, breakpoint] : this->hardwareBreakpointsByAddress) { - this->target->setHardwareBreakpoint(address); + for (const auto& [addressSpaceId, breakpointsByAddress] : this->hardwareBreakpointRegistry) { + for (const auto& [address, breakpoint] : breakpointsByAddress) { + this->target->setProgramBreakpoint(breakpoint); + } } auto newState = *(this->targetState); @@ -1022,34 +1102,26 @@ namespace TargetController return std::make_unique(); } - std::unique_ptr TargetControllerComponent::handleSetBreakpoint(SetBreakpoint& command) { - using Targets::TargetBreakpoint; - using Services::StringService; - - const auto& targetBreakpointResources = this->targetDescriptor->breakpointResources; - if ( - command.preferredType == Targets::TargetBreakpoint::Type::HARDWARE - && this->environmentConfig.targetConfig.hardwareBreakpoints - && ( - !targetBreakpointResources.maximumHardwareBreakpoints.has_value() - || this->hardwareBreakpointsByAddress.size() < (*(targetBreakpointResources.maximumHardwareBreakpoints) - - targetBreakpointResources.reservedHardwareBreakpoints) - ) - ) { - const auto hwBreakpoint = TargetBreakpoint{command.address, TargetBreakpoint::Type::HARDWARE}; - this->setBreakpoint(hwBreakpoint); - - return std::make_unique(hwBreakpoint); - } - - const auto swBreakpoint = TargetBreakpoint(command.address, TargetBreakpoint::Type::SOFTWARE); - this->setBreakpoint(swBreakpoint); - - return std::make_unique(swBreakpoint); + std::unique_ptr TargetControllerComponent::handleSetProgramBreakpointBreakpointAnyType( + SetProgramBreakpointAnyType& command + ) { + const 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 + }; + this->setProgramBreakpoint(breakpoint); + return std::make_unique(breakpoint); } - std::unique_ptr TargetControllerComponent::handleRemoveBreakpoint(RemoveBreakpoint& command) { - this->removeBreakpoint(command.breakpoint); + std::unique_ptr TargetControllerComponent::handleRemoveProgramBreakpoint( + RemoveProgramBreakpoint& command + ) { + this->removeProgramBreakpoint(command.breakpoint); return std::make_unique(); } diff --git a/src/TargetController/TargetControllerComponent.hpp b/src/TargetController/TargetControllerComponent.hpp index 8a42cb58..5cc2e30d 100644 --- a/src/TargetController/TargetControllerComponent.hpp +++ b/src/TargetController/TargetControllerComponent.hpp @@ -35,8 +35,8 @@ #include "Commands/WriteTargetMemory.hpp" #include "Commands/EraseTargetMemory.hpp" #include "Commands/StepTargetExecution.hpp" -#include "Commands/SetBreakpoint.hpp" -#include "Commands/RemoveBreakpoint.hpp" +#include "Commands/SetProgramBreakpointAnyType.hpp" +#include "Commands/RemoveProgramBreakpoint.hpp" #include "Commands/SetTargetProgramCounter.hpp" #include "Commands/SetTargetStackPointer.hpp" #include "Commands/GetTargetGpioPadStates.hpp" @@ -56,7 +56,7 @@ #include "Responses/TargetGpioPadStates.hpp" #include "Responses/TargetStackPointer.hpp" #include "Responses/TargetProgramCounter.hpp" -#include "Responses/Breakpoint.hpp" +#include "Responses/ProgramBreakpoint.hpp" #include "src/DebugToolDrivers/DebugTools.hpp" #include "src/Targets/BriefTargetDescriptor.hpp" @@ -66,6 +66,7 @@ #include "src/Targets/TargetMemory.hpp" #include "src/Targets/TargetAddressSpaceDescriptor.hpp" #include "src/Targets/TargetMemorySegmentDescriptor.hpp" +#include "src/Targets/ProgramBreakpointRegistry.hpp" #include "src/Targets/TargetMemoryCache.hpp" #include "src/EventManager/EventManager.hpp" @@ -156,11 +157,8 @@ namespace TargetController */ std::map registerDescriptorsByAddressSpaceKey; - /** - * The TargetController keeps track of all installed breakpoints. - */ - std::map softwareBreakpointsByAddress; - std::map hardwareBreakpointsByAddress; + Targets::ProgramBreakpointRegistry softwareBreakpointRegistry; + Targets::ProgramBreakpointRegistry hardwareBreakpointRegistry; /** * The target's program memory cache @@ -268,19 +266,14 @@ namespace TargetController void releaseHardware(); void startAtomicSession(); - void endActiveAtomicSession(); void refreshExecutionState(); - void updateTargetState(const Targets::TargetState& newState); void stopTarget(); - void resumeTarget(); - void stepTarget(); - void resetTarget(); Targets::TargetRegisterDescriptorAndValuePairs readTargetRegisters( @@ -310,9 +303,10 @@ namespace TargetController const Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor ); - void setBreakpoint(const Targets::TargetBreakpoint& breakpoint); - - void removeBreakpoint(const Targets::TargetBreakpoint& breakpoint); + std::uint32_t availableHardwareBreakpoints(); + void setProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint); + void removeProgramBreakpoint(const Targets::TargetProgramBreakpoint& breakpoint); + void clearAllBreakpoints(); /** * Puts the target into programming mode and disables command handlers for debug commands (commands that serve @@ -367,8 +361,10 @@ 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 handleRemoveBreakpoint(Commands::RemoveBreakpoint& command); + std::unique_ptr handleSetProgramBreakpointBreakpointAnyType( + Commands::SetProgramBreakpointAnyType& command + ); + std::unique_ptr handleRemoveProgramBreakpoint(Commands::RemoveProgramBreakpoint& command); std::unique_ptr handleSetProgramCounter(Commands::SetTargetProgramCounter& command); std::unique_ptr handleSetStackPointer(Commands::SetTargetStackPointer& command); std::unique_ptr handleGetTargetGpioPadStates( diff --git a/src/Targets/Microchip/AVR8/Avr8.cpp b/src/Targets/Microchip/AVR8/Avr8.cpp index 11f8547b..1f5238b0 100644 --- a/src/Targets/Microchip/AVR8/Avr8.cpp +++ b/src/Targets/Microchip/AVR8/Avr8.cpp @@ -25,8 +25,10 @@ namespace Targets::Microchip::Avr8 Avr8::Avr8(const TargetConfig& targetConfig, TargetDescriptionFile&& targetDescriptionFile) : targetConfig(Avr8TargetConfig{targetConfig}) , targetDescriptionFile(std::move(targetDescriptionFile)) + , programAddressSpaceDescriptor(this->targetDescriptionFile.getProgramAddressSpaceDescriptor()) , dataAddressSpaceDescriptor(this->targetDescriptionFile.getDataAddressSpaceDescriptor()) , fuseAddressSpaceDescriptor(this->targetDescriptionFile.getFuseAddressSpaceDescriptor()) + , programMemorySegmentDescriptor(this->targetDescriptionFile.getProgramMemorySegmentDescriptor()) , ramMemorySegmentDescriptor(this->targetDescriptionFile.getRamMemorySegmentDescriptor()) , ioMemorySegmentDescriptor(this->targetDescriptionFile.getIoMemorySegmentDescriptor()) , fuseMemorySegmentDescriptor(this->targetDescriptionFile.getFuseMemorySegmentDescriptor()) @@ -234,7 +236,6 @@ namespace Targets::Microchip::Avr8 } this->stop(); - this->clearAllBreakpoints(); if ( this->targetConfig.physicalInterface == TargetPhysicalInterface::JTAG @@ -357,24 +358,30 @@ namespace Targets::Microchip::Avr8 this->avr8DebugInterface->reset(); } - void Avr8::setSoftwareBreakpoint(TargetMemoryAddress address) { - this->avr8DebugInterface->setSoftwareBreakpoint(address); + void Avr8::setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.addressSpaceDescriptor != this->programAddressSpaceDescriptor) { + throw Exception{"Unexpected address space descriptor"}; + } + + if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { + this->avr8DebugInterface->setHardwareBreakpoint(breakpoint.address); + + } else { + this->avr8DebugInterface->setSoftwareBreakpoint(breakpoint.address); + } } - void Avr8::removeSoftwareBreakpoint(TargetMemoryAddress address) { - this->avr8DebugInterface->clearSoftwareBreakpoint(address); - } + void Avr8::removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if (breakpoint.addressSpaceDescriptor != this->programAddressSpaceDescriptor) { + throw Exception{"Unexpected address space descriptor"}; + } - void Avr8::setHardwareBreakpoint(TargetMemoryAddress address) { - this->avr8DebugInterface->setHardwareBreakpoint(address); - } + if (breakpoint.type == TargetProgramBreakpoint::Type::HARDWARE) { + this->avr8DebugInterface->clearHardwareBreakpoint(breakpoint.address); - void Avr8::removeHardwareBreakpoint(TargetMemoryAddress address) { - this->avr8DebugInterface->clearHardwareBreakpoint(address); - } - - void Avr8::clearAllBreakpoints() { - this->avr8DebugInterface->clearAllBreakpoints(); + } else { + this->avr8DebugInterface->clearSoftwareBreakpoint(breakpoint.address); + } } TargetRegisterDescriptorAndValuePairs Avr8::readRegisters(const Targets::TargetRegisterDescriptors& descriptors) { @@ -795,19 +802,19 @@ namespace Targets::Microchip::Avr8 } BreakpointResources Avr8::getBreakpointResources() { - auto maxHardwareBreakpoints = std::uint16_t{0}; + auto hardwareBreakpoints = std::uint32_t{0}; switch (this->targetConfig.physicalInterface) { case TargetPhysicalInterface::JTAG: { - maxHardwareBreakpoints = this->family == Family::XMEGA ? 2 : 3; + hardwareBreakpoints = this->family == Family::XMEGA ? 2 : 3; break; } case TargetPhysicalInterface::PDI: { - maxHardwareBreakpoints = 2; + hardwareBreakpoints = 2; break; } case TargetPhysicalInterface::UPDI: { - maxHardwareBreakpoints = 1; + hardwareBreakpoints = 1; break; } default: { @@ -816,11 +823,15 @@ namespace Targets::Microchip::Avr8 } return BreakpointResources{ - maxHardwareBreakpoints, - std::nullopt, + hardwareBreakpoints, + /* + * AVR targets have unlimited SW breakpoints, so we just use the program memory size divided by the + * breakpoint ("BREAK") opcode (byte) size, to determine the limit. + */ + this->programMemorySegmentDescriptor.size() / 2, std::min( - static_cast(this->targetConfig.reserveSteppingBreakpoint.value_or(true) ? 1 : 0), - maxHardwareBreakpoints + static_cast(this->targetConfig.reserveSteppingBreakpoint.value_or(true) ? 1 : 0), + hardwareBreakpoints ) }; } diff --git a/src/Targets/Microchip/AVR8/Avr8.hpp b/src/Targets/Microchip/AVR8/Avr8.hpp index 9c421398..175ccdd1 100644 --- a/src/Targets/Microchip/AVR8/Avr8.hpp +++ b/src/Targets/Microchip/AVR8/Avr8.hpp @@ -63,12 +63,8 @@ namespace Targets::Microchip::Avr8 void step() override; void reset() override; - void setSoftwareBreakpoint(TargetMemoryAddress address) override; - void removeSoftwareBreakpoint(TargetMemoryAddress address) override; - - void setHardwareBreakpoint(TargetMemoryAddress address) override; - void removeHardwareBreakpoint(TargetMemoryAddress address) override; - void clearAllBreakpoints() override; + void setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) override; + void removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) override; TargetRegisterDescriptorAndValuePairs readRegisters(const TargetRegisterDescriptors& descriptors) override; void writeRegisters(const TargetRegisterDescriptorAndValuePairs& registers) override; @@ -122,9 +118,11 @@ namespace Targets::Microchip::Avr8 Avr8TargetConfig targetConfig; TargetDescriptionFile targetDescriptionFile; + TargetAddressSpaceDescriptor programAddressSpaceDescriptor; TargetAddressSpaceDescriptor dataAddressSpaceDescriptor; TargetAddressSpaceDescriptor fuseAddressSpaceDescriptor; + TargetMemorySegmentDescriptor programMemorySegmentDescriptor; TargetMemorySegmentDescriptor ramMemorySegmentDescriptor; TargetMemorySegmentDescriptor ioMemorySegmentDescriptor; TargetMemorySegmentDescriptor fuseMemorySegmentDescriptor; diff --git a/src/Targets/Microchip/AVR8/TargetDescriptionFile.cpp b/src/Targets/Microchip/AVR8/TargetDescriptionFile.cpp index 73082009..0dc3551c 100644 --- a/src/Targets/Microchip/AVR8/TargetDescriptionFile.cpp +++ b/src/Targets/Microchip/AVR8/TargetDescriptionFile.cpp @@ -133,6 +133,10 @@ namespace Targets::Microchip::Avr8 return this->getLockbitAddressSpace().getMemorySegment("lockbits"); } + TargetAddressSpaceDescriptor TargetDescriptionFile::getProgramAddressSpaceDescriptor() const { + return this->targetAddressSpaceDescriptorFromAddressSpace(this->getProgramAddressSpace()); + } + TargetAddressSpaceDescriptor TargetDescriptionFile::getDataAddressSpaceDescriptor() const { return this->targetAddressSpaceDescriptorFromAddressSpace(this->getDataAddressSpace()); } @@ -141,6 +145,13 @@ namespace Targets::Microchip::Avr8 return this->targetAddressSpaceDescriptorFromAddressSpace(this->getFuseAddressSpace()); } + TargetMemorySegmentDescriptor TargetDescriptionFile::getProgramMemorySegmentDescriptor() const { + return this->targetMemorySegmentDescriptorFromMemorySegment( + this->getProgramMemorySegment(), + this->getProgramAddressSpace() + ); + } + TargetMemorySegmentDescriptor TargetDescriptionFile::getRamMemorySegmentDescriptor() const { return this->targetMemorySegmentDescriptorFromMemorySegment( this->getRamMemorySegment(), diff --git a/src/Targets/Microchip/AVR8/TargetDescriptionFile.hpp b/src/Targets/Microchip/AVR8/TargetDescriptionFile.hpp index 2e482f55..4d146d92 100644 --- a/src/Targets/Microchip/AVR8/TargetDescriptionFile.hpp +++ b/src/Targets/Microchip/AVR8/TargetDescriptionFile.hpp @@ -61,9 +61,11 @@ namespace Targets::Microchip::Avr8 [[nodiscard]] const TargetDescription::MemorySegment& getFuseMemorySegment() const; [[nodiscard]] const TargetDescription::MemorySegment& getLockbitMemorySegment() const; + [[nodiscard]] TargetAddressSpaceDescriptor getProgramAddressSpaceDescriptor() const; [[nodiscard]] TargetAddressSpaceDescriptor getDataAddressSpaceDescriptor() const; [[nodiscard]] TargetAddressSpaceDescriptor getFuseAddressSpaceDescriptor() const; + [[nodiscard]] TargetMemorySegmentDescriptor getProgramMemorySegmentDescriptor() const; [[nodiscard]] TargetMemorySegmentDescriptor getRamMemorySegmentDescriptor() const; [[nodiscard]] TargetMemorySegmentDescriptor getFuseMemorySegmentDescriptor() const; [[nodiscard]] TargetMemorySegmentDescriptor getIoMemorySegmentDescriptor() const; diff --git a/src/Targets/ProgramBreakpointRegistry.hpp b/src/Targets/ProgramBreakpointRegistry.hpp new file mode 100644 index 00000000..214648a5 --- /dev/null +++ b/src/Targets/ProgramBreakpointRegistry.hpp @@ -0,0 +1,112 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include "TargetBreakpoint.hpp" +#include "TargetMemory.hpp" + +namespace Targets +{ + /** + * Bookkeeping for program breakpoints. + * + * This template class will accept any type derived from the TargetProgramBreakpoint struct, to accommodate + * additional context-specific breakpoint data. + */ + template + requires std::is_base_of_v + class ProgramBreakpointRegistryGeneric + { + public: + std::unordered_map> mapping; + + void insert(const BreakpointType& breakpoint) { + this->mapping[breakpoint.addressSpaceDescriptor.id].emplace(breakpoint.address, breakpoint); + } + + void remove(TargetAddressSpaceId addressSpaceId, TargetMemoryAddress address) { + const auto addressMappingIt = this->mapping.find(addressSpaceId); + if (addressMappingIt == this->mapping.end()) { + return; + } + + addressMappingIt->second.erase(address); + } + + void remove(const TargetProgramBreakpoint& breakpoint) { + this->remove(breakpoint.addressSpaceDescriptor.id, breakpoint.address); + } + + std::optional> find( + TargetAddressSpaceId addressSpaceId, + TargetMemoryAddress address + ) const { + const auto addressMappingIt = this->mapping.find(addressSpaceId); + if (addressMappingIt == this->mapping.end()) { + return std::nullopt; + } + + const auto& addressMapping = addressMappingIt->second; + const auto breakpointIt = addressMapping.find(address); + if (breakpointIt == addressMapping.end()) { + return std::nullopt; + } + + return std::cref(breakpointIt->second); + } + + std::optional> find( + const TargetProgramBreakpoint& breakpoint + ) const { + return this->find(breakpoint.addressSpaceDescriptor.id, breakpoint.address); + } + + [[nodiscard]] bool contains(TargetAddressSpaceId addressSpaceId, TargetMemoryAddress address) const { + const auto addressMappingIt = this->mapping.find(addressSpaceId); + if (addressMappingIt == this->mapping.end()) { + return false; + } + + const auto& addressMapping = addressMappingIt->second; + const auto breakpointIt = addressMapping.find(address); + if (breakpointIt == addressMapping.end()) { + return false; + } + + return true; + } + + bool contains(const BreakpointType& breakpoint) const { + return this->contains(breakpoint.addressSpaceDescriptor.id, breakpoint.address); + } + + [[nodiscard]] std::size_t size() const { + return std::accumulate( + this->mapping.begin(), + this->mapping.end(), + std::size_t{0}, + [] (std::size_t accumulatedSize, const decltype(this->mapping)::value_type& addressMappingPair) { + return accumulatedSize + addressMappingPair.second.size(); + } + ); + } + + decltype(ProgramBreakpointRegistryGeneric::mapping)::const_iterator begin() const noexcept { + return this->mapping.begin(); + } + + decltype(ProgramBreakpointRegistryGeneric::mapping)::const_iterator end() const noexcept { + return this->mapping.end(); + } + + void clear() { + this->mapping.clear(); + } + }; + + using ProgramBreakpointRegistry = ProgramBreakpointRegistryGeneric; +} diff --git a/src/Targets/RiscV/Opcodes/Opcode.hpp b/src/Targets/RiscV/Opcodes/Opcode.hpp index 9f8b0977..1759a9db 100644 --- a/src/Targets/RiscV/Opcodes/Opcode.hpp +++ b/src/Targets/RiscV/Opcodes/Opcode.hpp @@ -5,6 +5,7 @@ namespace Targets::RiscV::Opcodes { using Opcode = std::uint32_t; + using OpcodeCompressed = std::uint16_t; enum class GprNumber: std::uint8_t { @@ -14,4 +15,8 @@ namespace Targets::RiscV::Opcodes }; static constexpr auto Ebreak = Opcode{0x00100073}; + static constexpr auto EbreakCompressed = OpcodeCompressed{0x9002}; + + static constexpr auto Fence = Opcode{0x0000000f}; + static constexpr auto FenceI = Opcode{0x0000100f}; } diff --git a/src/Targets/RiscV/ProgramBreakpoint.hpp b/src/Targets/RiscV/ProgramBreakpoint.hpp new file mode 100644 index 00000000..9ec246e9 --- /dev/null +++ b/src/Targets/RiscV/ProgramBreakpoint.hpp @@ -0,0 +1,23 @@ +#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/RiscV.cpp b/src/Targets/RiscV/RiscV.cpp index c7e899b6..a7608018 100644 --- a/src/Targets/RiscV/RiscV.cpp +++ b/src/Targets/RiscV/RiscV.cpp @@ -59,8 +59,6 @@ namespace Targets::RiscV this->stop(); } - this->clearAllBreakpoints(); - this->run(); this->riscVDebugInterface->deactivate(); } @@ -80,26 +78,6 @@ namespace Targets::RiscV this->riscVDebugInterface->reset(); } - void RiscV::setSoftwareBreakpoint(TargetMemoryAddress address) { - throw Exceptions::Exception{"TARGET - SW breakpoints not supported"}; - } - - void RiscV::removeSoftwareBreakpoint(TargetMemoryAddress address) { - throw Exceptions::Exception{"TARGET - SW breakpoints not supported"}; - } - - void RiscV::setHardwareBreakpoint(TargetMemoryAddress address) { - this->riscVDebugInterface->setHardwareBreakpoint(address); - } - - void RiscV::removeHardwareBreakpoint(TargetMemoryAddress address) { - this->riscVDebugInterface->clearHardwareBreakpoint(address); - } - - void RiscV::clearAllBreakpoints() { - this->riscVDebugInterface->clearAllHardwareBreakpoints(); - } - TargetRegisterDescriptorAndValuePairs RiscV::readRegisters(const TargetRegisterDescriptors& descriptors) { auto output = TargetRegisterDescriptorAndValuePairs{}; @@ -316,10 +294,12 @@ namespace Targets::RiscV } void RiscV::enableProgrammingMode() { + this->riscVDebugInterface->enableProgrammingMode(); this->programmingMode = true; } void RiscV::disableProgrammingMode() { + this->riscVDebugInterface->disableProgrammingMode(); this->programmingMode = false; } diff --git a/src/Targets/RiscV/RiscV.hpp b/src/Targets/RiscV/RiscV.hpp index dbeb61c6..1ae1fd51 100644 --- a/src/Targets/RiscV/RiscV.hpp +++ b/src/Targets/RiscV/RiscV.hpp @@ -38,13 +38,6 @@ namespace Targets::RiscV void step() override; void reset() override; - void setSoftwareBreakpoint(TargetMemoryAddress address) override; - void removeSoftwareBreakpoint(TargetMemoryAddress address) override; - - void setHardwareBreakpoint(TargetMemoryAddress address) override; - void removeHardwareBreakpoint(TargetMemoryAddress address) override; - void clearAllBreakpoints() override; - TargetRegisterDescriptorAndValuePairs readRegisters(const TargetRegisterDescriptors& descriptors) override; void writeRegisters(const TargetRegisterDescriptorAndValuePairs& registers) override; @@ -84,9 +77,7 @@ namespace Targets::RiscV void setGpioPadState(const TargetPadDescriptor& padDescriptor, const TargetGpioPadState& state) override; void enableProgrammingMode() override; - void disableProgrammingMode() override; - bool programmingModeEnabled() override; protected: diff --git a/src/Targets/RiscV/Wch/WchRiscV.cpp b/src/Targets/RiscV/Wch/WchRiscV.cpp index 8743784b..c0be949f 100644 --- a/src/Targets/RiscV/Wch/WchRiscV.cpp +++ b/src/Targets/RiscV/Wch/WchRiscV.cpp @@ -47,8 +47,6 @@ namespace Targets::RiscV::Wch } TargetDescriptor WchRiscV::targetDescriptor() { - const auto hardwareBreakpointCount = this->riscVDebugInterface->getHardwareBreakpointCount(); - auto descriptor = TargetDescriptor{ this->targetDescriptionFile.getName(), this->targetDescriptionFile.getFamily(), @@ -59,15 +57,16 @@ namespace Targets::RiscV::Wch this->targetDescriptionFile.targetPadDescriptorsByKey(), this->targetDescriptionFile.targetPinoutDescriptorsByKey(), this->targetDescriptionFile.targetVariantDescriptorsByKey(), - BreakpointResources{ - hardwareBreakpointCount, - std::nullopt, - static_cast( - this->targetConfig.reserveSteppingBreakpoint.value_or(false) && hardwareBreakpointCount > 0 ? 1 : 0 - ) - } + this->riscVDebugInterface->getBreakpointResources() }; + if ( + this->targetConfig.reserveSteppingBreakpoint.value_or(false) + && descriptor.breakpointResources.hardwareBreakpoints > 0 + ) { + descriptor.breakpointResources.reservedHardwareBreakpoints = 1; + } + // Copy the RISC-V CPU register address space and peripheral descriptor descriptor.addressSpaceDescriptorsByKey.emplace( this->cpuRegisterAddressSpaceDescriptor.key, @@ -100,6 +99,44 @@ namespace Targets::RiscV::Wch return descriptor; } + void WchRiscV::setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if ( + breakpoint.type == TargetProgramBreakpoint::Type::SOFTWARE + && breakpoint.memorySegmentDescriptor == this->mappedProgramMemorySegmentDescriptor + ) { + this->riscVDebugInterface->setProgramBreakpoint(TargetProgramBreakpoint{ + .addressSpaceDescriptor = this->sysAddressSpaceDescriptor, + .memorySegmentDescriptor = this->getDestinationProgramMemorySegmentDescriptor(), + .address = this->transformAliasedProgramMemoryAddress(breakpoint.address), + .size = breakpoint.size, + .type = breakpoint.type + }); + + return; + } + + this->riscVDebugInterface->setProgramBreakpoint(breakpoint); + } + + void WchRiscV::removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) { + if ( + breakpoint.type == TargetProgramBreakpoint::Type::SOFTWARE + && breakpoint.memorySegmentDescriptor == this->mappedProgramMemorySegmentDescriptor + ) { + this->riscVDebugInterface->removeProgramBreakpoint(TargetProgramBreakpoint{ + .addressSpaceDescriptor = this->sysAddressSpaceDescriptor, + .memorySegmentDescriptor = this->getDestinationProgramMemorySegmentDescriptor(), + .address = this->transformAliasedProgramMemoryAddress(breakpoint.address), + .size = breakpoint.size, + .type = breakpoint.type + }); + + return; + } + + this->riscVDebugInterface->removeProgramBreakpoint(breakpoint); + } + void WchRiscV::writeMemory( const TargetAddressSpaceDescriptor& addressSpaceDescriptor, const TargetMemorySegmentDescriptor& memorySegmentDescriptor, @@ -119,13 +156,35 @@ namespace Targets::RiscV::Wch * before v2.0.0. */ if (memorySegmentDescriptor == this->mappedProgramMemorySegmentDescriptor) { - const auto newAddress = startAddress - this->mappedProgramMemorySegmentDescriptor.addressRange.startAddress - + this->programMemorySegmentDescriptor.addressRange.startAddress; - assert(this->programMemorySegmentDescriptor.addressRange.contains(newAddress)); + const auto transformedAddress = this->transformAliasedProgramMemoryAddress(startAddress); + assert(this->programMemorySegmentDescriptor.addressRange.contains(transformedAddress)); - return RiscV::writeMemory(addressSpaceDescriptor, this->programMemorySegmentDescriptor, newAddress, buffer); + return RiscV::writeMemory( + addressSpaceDescriptor, + this->programMemorySegmentDescriptor, + transformedAddress, + buffer + ); } return RiscV::writeMemory(addressSpaceDescriptor, memorySegmentDescriptor, startAddress, buffer); } + + const TargetMemorySegmentDescriptor& WchRiscV::getDestinationProgramMemorySegmentDescriptor() { + return this->programMemorySegmentDescriptor; + } + + TargetMemoryAddress WchRiscV::transformAliasedProgramMemoryAddress(TargetMemoryAddress address) const { + using Services::StringService; + + const auto transformedAddress = address - this->mappedProgramMemorySegmentDescriptor.addressRange.startAddress + + this->programMemorySegmentDescriptor.addressRange.startAddress; + + Logger::debug( + "Transformed mapped program memory address 0x" + StringService::toHex(address) + " to 0x" + + StringService::toHex(transformedAddress) + ); + + return transformedAddress; + } } diff --git a/src/Targets/RiscV/Wch/WchRiscV.hpp b/src/Targets/RiscV/Wch/WchRiscV.hpp index 5f990890..13bbcfe3 100644 --- a/src/Targets/RiscV/Wch/WchRiscV.hpp +++ b/src/Targets/RiscV/Wch/WchRiscV.hpp @@ -19,6 +19,9 @@ namespace Targets::RiscV::Wch void postActivate() override; TargetDescriptor targetDescriptor() override; + void setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) override; + void removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) override; + void writeMemory( const TargetAddressSpaceDescriptor& addressSpaceDescriptor, const TargetMemorySegmentDescriptor& memorySegmentDescriptor, @@ -31,6 +34,10 @@ namespace Targets::RiscV::Wch std::optional> variant = std::nullopt; const TargetMemorySegmentDescriptor& programMemorySegmentDescriptor; + const TargetMemorySegmentDescriptor& bootProgramMemorySegmentDescriptor; const TargetMemorySegmentDescriptor& mappedProgramMemorySegmentDescriptor; + + const TargetMemorySegmentDescriptor& getDestinationProgramMemorySegmentDescriptor(); + TargetMemoryAddress transformAliasedProgramMemoryAddress(TargetMemoryAddress address) const; }; } diff --git a/src/Targets/Target.hpp b/src/Targets/Target.hpp index 5a6dbd58..035df579 100644 --- a/src/Targets/Target.hpp +++ b/src/Targets/Target.hpp @@ -116,40 +116,8 @@ namespace Targets */ virtual void reset() = 0; - /** - * Should set a software breakpoint on the target, at the given address. - * - * @param address - */ - virtual void setSoftwareBreakpoint(TargetMemoryAddress address) = 0; - - /** - * Should remove a software breakpoint at the given address. - * - * @param address - */ - 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. - * - * @TODO: is this still needed? Review - */ - virtual void clearAllBreakpoints() = 0; + virtual void setProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) = 0; + virtual void removeProgramBreakpoint(const TargetProgramBreakpoint& breakpoint) = 0; /** * Should read register values of the registers described by the given descriptors. diff --git a/src/Targets/TargetAddressSpaceDescriptor.cpp b/src/Targets/TargetAddressSpaceDescriptor.cpp index e7c62c7b..667b2b9d 100644 --- a/src/Targets/TargetAddressSpaceDescriptor.cpp +++ b/src/Targets/TargetAddressSpaceDescriptor.cpp @@ -85,6 +85,18 @@ namespace Targets return output; } + std::optional< + std::reference_wrapper + > TargetAddressSpaceDescriptor::getContainingMemorySegmentDescriptor(const TargetMemoryAddress& address) const { + for (const auto& [key, segmentDescriptor] : this->segmentDescriptorsByKey) { + if (segmentDescriptor.addressRange.contains(address)) { + return std::cref(segmentDescriptor); + } + } + + return std::nullopt; + } + TargetAddressSpaceDescriptor TargetAddressSpaceDescriptor::clone() const { auto output = TargetAddressSpaceDescriptor{ this->key, diff --git a/src/Targets/TargetAddressSpaceDescriptor.hpp b/src/Targets/TargetAddressSpaceDescriptor.hpp index d5b9654e..0e1e9545 100644 --- a/src/Targets/TargetAddressSpaceDescriptor.hpp +++ b/src/Targets/TargetAddressSpaceDescriptor.hpp @@ -92,6 +92,18 @@ namespace Targets const TargetMemoryAddressRange& addressRange ) const; + /** + * Fetches the memory segment which contains the given address. + * + * @param address + * + * @return + * The containing memory segment descriptor, or std::nullopt of no segments contain the address. + */ + [[nodiscard]] std::optional< + std::reference_wrapper + > getContainingMemorySegmentDescriptor(const TargetMemoryAddress& address) const; + [[nodiscard]] TargetAddressSpaceDescriptor clone() const; static TargetAddressSpaceId generateId(const std::string& addressSpaceKey); diff --git a/src/Targets/TargetBreakpoint.hpp b/src/Targets/TargetBreakpoint.hpp index 999ed93b..2aa3d57a 100644 --- a/src/Targets/TargetBreakpoint.hpp +++ b/src/Targets/TargetBreakpoint.hpp @@ -5,6 +5,8 @@ #include #include "TargetMemory.hpp" +#include "TargetAddressSpaceDescriptor.hpp" +#include "TargetMemorySegmentDescriptor.hpp" namespace Targets { @@ -14,7 +16,7 @@ namespace Targets UNKNOWN, }; - struct TargetBreakpoint + struct TargetProgramBreakpoint { enum class Type: std::uint8_t { @@ -22,42 +24,17 @@ namespace Targets SOFTWARE, }; - /** - * Byte address of the breakpoint. - */ - TargetMemoryAddress address = 0; - - Type type = Type::SOFTWARE; - - TargetBreakpoint() = default; - - explicit TargetBreakpoint(TargetMemoryAddress address, Type type = Type::SOFTWARE) - : address(address) - , type(type) - {}; + const TargetAddressSpaceDescriptor& addressSpaceDescriptor; + const TargetMemorySegmentDescriptor& memorySegmentDescriptor; + TargetMemoryAddress address; + TargetMemorySize size; + Type type; }; struct BreakpointResources { - std::optional maximumHardwareBreakpoints = std::nullopt; - std::optional maximumSoftwareBreakpoints = std::nullopt; - std::uint16_t reservedHardwareBreakpoints = 0; - - BreakpointResources() = default; - - BreakpointResources( - std::optional maximumHardwareBreakpoints, - std::optional maximumSoftwareBreakpoints, - std::uint16_t reservedHardwareBreakpoints - ) - : maximumHardwareBreakpoints(maximumHardwareBreakpoints) - , maximumSoftwareBreakpoints(maximumSoftwareBreakpoints) - , reservedHardwareBreakpoints(reservedHardwareBreakpoints) - { - assert( - !this->maximumHardwareBreakpoints.has_value() - || this->maximumHardwareBreakpoints >= this->reservedHardwareBreakpoints - ); - } + std::uint32_t hardwareBreakpoints = 0; + std::uint32_t softwareBreakpoints = 0; + std::uint32_t reservedHardwareBreakpoints = 0; }; } diff --git a/src/Targets/TargetDescriptor.cpp b/src/Targets/TargetDescriptor.cpp index 15b119a3..952acf24 100644 --- a/src/Targets/TargetDescriptor.cpp +++ b/src/Targets/TargetDescriptor.cpp @@ -42,6 +42,18 @@ namespace Targets return std::cref(descriptorIt->second); } + std::optional< + std::reference_wrapper + > TargetDescriptor::tryGetAddressSpaceDescriptor(const std::string& key) { + const auto descriptorIt = this->addressSpaceDescriptorsByKey.find(key); + + if (descriptorIt == this->addressSpaceDescriptorsByKey.end()) { + return std::nullopt; + } + + return std::ref(descriptorIt->second); + } + const TargetAddressSpaceDescriptor& TargetDescriptor::getAddressSpaceDescriptor(const std::string& key) const { const auto descriptor = this->tryGetAddressSpaceDescriptor(key); if (!descriptor.has_value()) { @@ -54,6 +66,28 @@ namespace Targets return descriptor->get(); } + TargetAddressSpaceDescriptor& TargetDescriptor::getAddressSpaceDescriptor(const std::string& key) { + return const_cast( + const_cast(this)->getAddressSpaceDescriptor(key) + ); + } + + const TargetMemorySegmentDescriptor& TargetDescriptor::getMemorySegmentDescriptor( + const std::string& addressSpaceKey, + const std::string& segmentKey + ) const { + return this->getAddressSpaceDescriptor(addressSpaceKey).getMemorySegmentDescriptor(segmentKey); + } + + TargetMemorySegmentDescriptor& TargetDescriptor::getMemorySegmentDescriptor( + const std::string& addressSpaceKey, + const std::string& segmentKey + ) { + return const_cast( + const_cast(this)->getMemorySegmentDescriptor(addressSpaceKey, segmentKey) + ); + } + const TargetAddressSpaceDescriptor& TargetDescriptor::getFirstAddressSpaceDescriptorContainingMemorySegment( const std::string& memorySegmentKey ) const { diff --git a/src/Targets/TargetDescriptor.hpp b/src/Targets/TargetDescriptor.hpp index d5a4418a..1438b233 100644 --- a/src/Targets/TargetDescriptor.hpp +++ b/src/Targets/TargetDescriptor.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -53,8 +54,21 @@ namespace Targets std::optional> tryGetAddressSpaceDescriptor( const std::string& key ) const; + std::optional> tryGetAddressSpaceDescriptor( + const std::string& key + ); const TargetAddressSpaceDescriptor& getAddressSpaceDescriptor(const std::string& key) const; + TargetAddressSpaceDescriptor& getAddressSpaceDescriptor(const std::string& key); + + const TargetMemorySegmentDescriptor& getMemorySegmentDescriptor( + const std::string& addressSpaceKey, + const std::string& segmentKey + ) const; + TargetMemorySegmentDescriptor& getMemorySegmentDescriptor( + const std::string& addressSpaceKey, + const std::string& segmentKey + ); /** * Returns the descriptor for the first address space that contains the given memory segment.