From 7aff716d5de261e728a464babcea2d0079fda6ac Mon Sep 17 00:00:00 2001 From: Nav Date: Sun, 25 May 2025 17:08:01 +0100 Subject: [PATCH] More tidying --- .../RiscVGdb/CommandPackets/ReadMemoryMap.cpp | 29 ++++++++++++------- .../Protocols/RiscVDebug/DebugTranslator.cpp | 25 ++++------------ .../TriggerModule/Registers/TriggerInfo.hpp | 4 +-- .../TriggerModule/Registers/TriggerSelect.hpp | 24 --------------- .../TargetControllerComponent.cpp | 11 +++---- 5 files changed, 33 insertions(+), 60 deletions(-) delete mode 100644 src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerSelect.hpp diff --git a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/ReadMemoryMap.cpp b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/ReadMemoryMap.cpp index 7e019ee1..abd43f3c 100644 --- a/src/DebugServer/Gdb/RiscVGdb/CommandPackets/ReadMemoryMap.cpp +++ b/src/DebugServer/Gdb/RiscVGdb/CommandPackets/ReadMemoryMap.cpp @@ -3,7 +3,6 @@ #include "src/DebugServer/Gdb/ResponsePackets/ResponsePacket.hpp" #include "src/Services/StringService.hpp" -#include "src/Helpers/BiMap.hpp" #include "src/Exceptions/Exception.hpp" namespace DebugServer::Gdb::RiscVGdb::CommandPackets @@ -48,24 +47,34 @@ namespace DebugServer::Gdb::RiscVGdb::CommandPackets Logger::info("Handling ReadMemoryMap packet"); - static const auto gdbMemoryTypesBySegmentType = BiMap{ - {TargetMemorySegmentType::FLASH, "flash"}, - {TargetMemorySegmentType::RAM, "ram"}, - {TargetMemorySegmentType::IO, "ram"}, - {TargetMemorySegmentType::ALIASED, "flash"}, // TODO: Assumption made here. Will hold for now. Review later + static const auto gdbMemoryTypeFromSegment = [] ( + const Targets::TargetMemorySegmentDescriptor& segmentDescriptor + ) -> std::optional { + switch (segmentDescriptor.type) { + case TargetMemorySegmentType::FLASH: + case TargetMemorySegmentType::ALIASED: { + return "flash"; + } + case TargetMemorySegmentType::RAM: + case TargetMemorySegmentType::IO: { + return "ram"; + } + default: { + return std::nullopt; + } + } }; auto memoryMap = std::string{"\n"}; - for (const auto& [segmentKey, segmentDescriptor] : gdbTargetDescriptor.systemAddressSpaceDescriptor.segmentDescriptorsByKey) { - const auto gdbMemType = gdbMemoryTypesBySegmentType.valueAt(segmentDescriptor.type); + for (const auto& segmentDescriptor : gdbTargetDescriptor.systemAddressSpaceDescriptor.segmentDescriptorsByKey | std::views::values) { + const auto gdbMemType = gdbMemoryTypeFromSegment(segmentDescriptor); if (!gdbMemType.has_value()) { continue; } const auto segmentWritable = ( - segmentDescriptor.debugModeAccess.writeable - || segmentDescriptor.programmingModeAccess.writeable + segmentDescriptor.debugModeAccess.writeable || segmentDescriptor.programmingModeAccess.writeable ); memoryMap += "debugModuleDescriptor.hartIndices.size() > 1) { - Logger::debug( - "Discovered RISC-V harts: " + std::to_string(this->debugModuleDescriptor.hartIndices.size()) - ); + Logger::debug("Discovered RISC-V harts: " + std::to_string(this->debugModuleDescriptor.hartIndices.size())); Logger::warning("Bloom only supports debugging a single RISC-V hart - selecting first available"); } @@ -353,11 +350,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug if (triggerDescriptor.supportedTypes.contains(TriggerType::MATCH_CONTROL)) { using TriggerModule::Registers::MatchControl; - this->writeCpuRegister( - CpuRegisterNumber::TRIGGER_SELECT, - TriggerModule::Registers::TriggerSelect{triggerDescriptor.index}.value() - ); - + this->writeCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, triggerDescriptor.index); this->writeCpuRegister( CpuRegisterNumber::TRIGGER_DATA_1, MatchControl{ @@ -650,9 +643,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug static constexpr auto MAX_TRIGGER_INDEX = 10; for (auto triggerIndex = TriggerModule::TriggerIndex{0}; triggerIndex <= MAX_TRIGGER_INDEX; ++triggerIndex) { - const auto selectRegValue = TriggerModule::Registers::TriggerSelect{triggerIndex}.value(); - - const auto writeSelectError = this->tryWriteCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, selectRegValue); + const auto writeSelectError = this->tryWriteCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, triggerIndex); if (writeSelectError == AbstractCommandError::EXCEPTION) { break; } @@ -664,7 +655,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug }; } - if (this->readCpuRegister(CpuRegisterNumber::TRIGGER_SELECT) != selectRegValue) { + if (this->readCpuRegister(CpuRegisterNumber::TRIGGER_SELECT) != triggerIndex) { break; } @@ -786,7 +777,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug .registerNumber = number, .transfer = true, .flags = flags, - .size= RegisterAccessControlField::RegisterSize::SIZE_32 + .size = RegisterAccessControlField::RegisterSize::SIZE_32 }.value(), .commandType = AbstractCommandRegister::CommandType::REGISTER_ACCESS }); @@ -1315,11 +1306,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug if (triggerDescriptor.supportedTypes.contains(TriggerType::MATCH_CONTROL)) { using TriggerModule::Registers::MatchControl; - this->writeCpuRegister( - CpuRegisterNumber::TRIGGER_SELECT, - TriggerModule::Registers::TriggerSelect{triggerDescriptor.index}.value() - ); - + this->writeCpuRegister(CpuRegisterNumber::TRIGGER_SELECT, triggerDescriptor.index); this->writeCpuRegister(CpuRegisterNumber::TRIGGER_DATA_1, MatchControl{}.value()); return; } diff --git a/src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerInfo.hpp b/src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerInfo.hpp index 08b5405c..67998b8a 100644 --- a/src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerInfo.hpp +++ b/src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerInfo.hpp @@ -30,7 +30,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug::TriggerModule::Registers [[nodiscard]] std::set getSupportedTriggerTypes() const { auto output = std::set{}; - static constexpr auto types = std::to_array({ + static constexpr auto TYPES = std::to_array({ TriggerType::LEGACY, TriggerType::MATCH_CONTROL, TriggerType::INSTRUCTION_COUNT, @@ -40,7 +40,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebug::TriggerModule::Registers TriggerType::EXTERNAL, }); - for (const auto& type : types) { + for (const auto& type : TYPES) { if (this->info & (0x01 << static_cast(type))) { output.insert(type); } diff --git a/src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerSelect.hpp b/src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerSelect.hpp deleted file mode 100644 index baef0f88..00000000 --- a/src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/Registers/TriggerSelect.hpp +++ /dev/null @@ -1,24 +0,0 @@ -#pragma once - -#include - -#include "src/DebugToolDrivers/Protocols/RiscVDebug/TriggerModule/TriggerModule.hpp" - -namespace DebugToolDrivers::Protocols::RiscVDebug::TriggerModule::Registers -{ - /** - * TODO: Given the single, full width bit field, is this struct really necessary? Review. - */ - struct TriggerSelect - { - TriggerIndex index; - - constexpr explicit TriggerSelect(TriggerIndex index) - : index(index) - {} - - [[nodiscard]] constexpr RegisterValue value() const { - return static_cast(this->index); - } - }; -} diff --git a/src/TargetController/TargetControllerComponent.cpp b/src/TargetController/TargetControllerComponent.cpp index 3339e8ef..6e09adba 100644 --- a/src/TargetController/TargetControllerComponent.cpp +++ b/src/TargetController/TargetControllerComponent.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "src/Targets/Microchip/Avr8/TargetDescriptionFile.hpp" #include "src/Targets/RiscV/Wch/TargetDescriptionFile.hpp" @@ -1046,7 +1047,7 @@ namespace TargetController }; auto commitOperations = std::vector{}; - for (const auto& [segmentId, writeOperation] : session.writeOperationsBySegmentId) { + for (const auto& writeOperation : session.writeOperationsBySegmentId | std::views::values) { auto& segmentCache = this->getProgramMemoryCache(writeOperation.memorySegmentDescriptor); // Can the program memory cache facilitate diffing with all regions in this write operation? @@ -1101,8 +1102,8 @@ namespace TargetController * before constructing the delta segments. */ auto cacheData = segmentCache.data; - for (const auto& [addressSpaceId, breakpointsByAddress] : this->softwareBreakpointRegistry) { - for (const auto& [address, breakpoint] : breakpointsByAddress) { + for (const auto& breakpointsByAddress : this->softwareBreakpointRegistry | std::views::values) { + for (const auto& breakpoint : breakpointsByAddress | std::views::values) { if (breakpoint.memorySegmentDescriptor != writeOperation.memorySegmentDescriptor) { continue; } @@ -1169,11 +1170,11 @@ namespace TargetController } void TargetControllerComponent::abandonDeltaProgrammingSession(const DeltaProgramming::Session& session) { - for (const auto& [segmentId, eraseOperation] : session.eraseOperationsBySegmentId) { + for (const auto& eraseOperation: session.eraseOperationsBySegmentId | std::views::values) { this->eraseTargetMemory(eraseOperation.addressSpaceDescriptor, eraseOperation.memorySegmentDescriptor); } - for (const auto& [segmentId, writeOperation] : session.writeOperationsBySegmentId) { + for (const auto& writeOperation : session.writeOperationsBySegmentId | std::views::values) { for (const auto& region : writeOperation.regions) { this->writeTargetMemory( writeOperation.addressSpaceDescriptor,