From 03e6152157ae5b62b59b54291dc2ecfbd354056a Mon Sep 17 00:00:00 2001 From: Nav Date: Sun, 2 Feb 2025 00:03:35 +0000 Subject: [PATCH] Tidying --- CMakeLists.txt | 4 +- .../Protocols/Edbg/Avr/EdbgAvr8Interface.cpp | 7 +- .../RiscVDebugSpec/DebugTranslator.cpp | 89 ++++++++++--------- .../RiscVDebugSpec/DebugTranslator.hpp | 6 -- .../Wch/WchLinkDebugInterface.hpp | 6 +- 5 files changed, 53 insertions(+), 59 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b3b1e33b..390c278b 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -131,8 +131,8 @@ target_compile_options( PUBLIC -Wsuggest-override PUBLIC -Wreorder PUBLIC -fno-sized-deallocation - PUBLIC -ffile-prefix-map=${CMAKE_SOURCE_DIR}/./src/= - PUBLIC -ffile-prefix-map=${CMAKE_SOURCE_DIR}/src/= + PUBLIC $<$:-ffile-prefix-map=${CMAKE_SOURCE_DIR}/./src/=> + PUBLIC $<$:-ffile-prefix-map=${CMAKE_SOURCE_DIR}/src/=> PUBLIC $<$:-g> # PUBLIC $<$:-O0> PUBLIC $<$:-Ofast> diff --git a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp index 5f389e57..3bfbf274 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp +++ b/src/DebugToolDrivers/Microchip/Protocols/Edbg/Avr/EdbgAvr8Interface.cpp @@ -7,6 +7,7 @@ #include #include "src/Services/PathService.hpp" +#include "src/Services/AlignmentService.hpp" #include "src/Services/StringService.hpp" #include "src/Logger/Logger.hpp" @@ -1537,7 +1538,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } if ((address % alignTo) != 0) { - return (address / alignTo) * alignTo; + return Services::AlignmentService::alignMemoryAddress(address, alignTo); } return address; @@ -1567,9 +1568,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr } if ((bytes % alignTo) != 0) { - return static_cast(std::ceil( - static_cast(bytes) / static_cast(alignTo) - ) * alignTo); + return Services::AlignmentService::alignMemorySize(bytes, alignTo); } return bytes; diff --git a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp index 33bbea41..306f1036 100644 --- a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp +++ b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.cpp @@ -25,6 +25,7 @@ #include "TriggerModule/Registers/TriggerData1.hpp" #include "TriggerModule/Registers/MatchControl.hpp" +#include "src/Services/AlignmentService.hpp" #include "src/Helpers/Array.hpp" #include "src/Exceptions/Exception.hpp" @@ -137,7 +138,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec * Attempt to read a single word from the start of the system address space, via a memory access abstract * command. */ - constexpr auto probingMemoryAccessCommand = AbstractCommandRegister{ + static constexpr auto PROBE_ACCESS_COMMAND = AbstractCommandRegister{ .control = MemoryAccessControlField{ .postIncrement = true, .size = MemoryAccessControlField::MemorySize::SIZE_32, @@ -150,7 +151,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec this->targetDescriptionFile.getSystemAddressSpace().startAddress ); - if (this->tryExecuteAbstractCommand(probingMemoryAccessCommand) == AbstractCommandError::NONE) { + if (this->tryExecuteAbstractCommand(PROBE_ACCESS_COMMAND) == AbstractCommandError::NONE) { this->debugModuleDescriptor.memoryAccessStrategies.insert(MemoryAccessStrategy::ABSTRACT_COMMAND); } } @@ -443,18 +444,24 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec TargetMemorySize bytes, const std::set& excludedAddressRanges ) { + using Services::AlignmentService; + if (addressSpaceDescriptor != this->sysAddressSpaceDescriptor) { throw Exceptions::TargetOperationFailure{"Unsupported address space"}; } // TODO: excluded addresses - constexpr auto alignTo = DebugTranslator::WORD_BYTE_SIZE; - if ((startAddress % alignTo) != 0 || (bytes % alignTo) != 0) { - // Alignment required - const auto alignedStartAddress = this->alignMemoryAddress(startAddress, alignTo); - const auto alignedBytes = this->alignMemorySize(bytes + (startAddress - alignedStartAddress), alignTo); + const auto alignedStartAddress = AlignmentService::alignMemoryAddress( + startAddress, + DebugTranslator::WORD_BYTE_SIZE + ); + const auto alignedBytes = AlignmentService::alignMemorySize( + bytes + (startAddress - alignedStartAddress), + DebugTranslator::WORD_BYTE_SIZE + ); + if (alignedStartAddress != startAddress || alignedBytes != bytes) { const auto memoryBuffer = this->readMemory( addressSpaceDescriptor, memorySegmentDescriptor, @@ -484,22 +491,24 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec TargetMemoryAddress startAddress, TargetMemoryBufferSpan buffer ) { + using Services::AlignmentService; + if (addressSpaceDescriptor != this->sysAddressSpaceDescriptor) { throw Exceptions::TargetOperationFailure{"Unsupported address space"}; } - constexpr auto alignTo = DebugTranslator::WORD_BYTE_SIZE; const auto bytes = static_cast(buffer.size()); - if ((startAddress % alignTo) != 0 || (bytes % alignTo) != 0) { - /* - * Alignment required - * - * To align the write operation, we read the front and back offset bytes and use them to construct an - * aligned buffer. - */ - const auto alignedStartAddress = this->alignMemoryAddress(startAddress, alignTo); - const auto alignedBytes = this->alignMemorySize(bytes + (startAddress - alignedStartAddress), alignTo); + const auto alignedStartAddress = AlignmentService::alignMemoryAddress( + startAddress, + DebugTranslator::WORD_BYTE_SIZE + ); + const auto alignedBytes = AlignmentService::alignMemorySize( + bytes + (startAddress - alignedStartAddress), + DebugTranslator::WORD_BYTE_SIZE + ); + + if (alignedStartAddress != startAddress || alignedBytes != bytes) { assert(alignedBytes > bytes); auto alignedBuffer = (alignedStartAddress < startAddress) @@ -566,26 +575,28 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec return; } - static constexpr auto clearedBuffer = Array::repeat(Opcodes::Ebreak); + static constexpr auto CLEARED_BUFFER = Array::repeat( + Opcodes::Ebreak + ); assert(this->debugModuleDescriptor.programBufferSize <= DebugTranslator::MAX_PROGRAM_BUFFER_SIZE); - this->writeProgramBuffer({clearedBuffer.begin(), this->debugModuleDescriptor.programBufferSize}); + this->writeProgramBuffer({CLEARED_BUFFER.begin(), this->debugModuleDescriptor.programBufferSize}); } void DebugTranslator::executeFenceProgram() { - static constexpr auto programOpcodes = std::to_array({ + static constexpr auto PROGRAM_OPCODES = std::to_array({ Opcodes::FenceI, Opcodes::Fence, Opcodes::Ebreak, }); - if (programOpcodes.size() > this->debugModuleDescriptor.programBufferSize) { + if (PROGRAM_OPCODES.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->writeProgramBuffer(PROGRAM_OPCODES); this->readCpuRegister(CpuRegisterNumber::GPR_X8, {.postExecute = true}); } @@ -634,7 +645,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec > DebugTranslator::discoverTriggers() { auto output = std::unordered_map{}; - constexpr auto MAX_TRIGGER_INDEX = 10; + 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(); @@ -943,16 +954,6 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec : *(this->debugModuleDescriptor.memoryAccessStrategies.begin()); } - TargetMemoryAddress DebugTranslator::alignMemoryAddress(TargetMemoryAddress address, TargetMemoryAddress alignTo) { - return (address / alignTo) * alignTo; - } - - TargetMemorySize DebugTranslator::alignMemorySize(TargetMemorySize size, TargetMemorySize alignTo) { - return static_cast( - std::ceil(static_cast(size) / static_cast(alignTo)) - ) * alignTo; - } - Targets::TargetMemoryBuffer DebugTranslator::readMemoryViaAbstractCommand( Targets::TargetMemoryAddress startAddress, Targets::TargetMemorySize bytes @@ -966,7 +967,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec */ this->dtmInterface.writeDebugModuleRegister(RegisterAddress::ABSTRACT_DATA_1, startAddress); - constexpr auto command = AbstractCommandRegister{ + static constexpr auto COMMAND = AbstractCommandRegister{ .control = MemoryAccessControlField{ .postIncrement = true, .size = MemoryAccessControlField::MemorySize::SIZE_32, @@ -978,7 +979,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec output.reserve(bytes); for (auto address = startAddress; address <= (startAddress + bytes - 1); address += 4) { - const auto commandError = this->tryExecuteAbstractCommand(command); + const auto commandError = this->tryExecuteAbstractCommand(COMMAND); if (commandError != AbstractCommandError::NONE) { if (commandError == AbstractCommandError::EXCEPTION) { throw Exceptions::IllegalMemoryAccess{}; @@ -1010,7 +1011,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec this->dtmInterface.writeDebugModuleRegister(RegisterAddress::ABSTRACT_DATA_1, startAddress); - static constexpr auto command = AbstractCommandRegister{ + static constexpr auto COMMAND = AbstractCommandRegister{ .control = MemoryAccessControlField{ .write = true, .postIncrement = true, @@ -1030,7 +1031,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec ) ); - const auto commandError = this->tryExecuteAbstractCommand(command); + const auto commandError = this->tryExecuteAbstractCommand(COMMAND); if (commandError != AbstractCommandError::NONE) { if (commandError == AbstractCommandError::EXCEPTION) { throw Exceptions::IllegalMemoryAccess{}; @@ -1051,7 +1052,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec assert(startAddress % DebugTranslator::WORD_BYTE_SIZE == 0); assert(bytes % DebugTranslator::WORD_BYTE_SIZE == 0); - static constexpr auto programOpcodes = std::to_array({ + static constexpr auto PROGRAM_OPCODES = std::to_array({ Opcodes::Lw{ .destinationRegister = Opcodes::GprNumber::X9, .baseAddressRegister = Opcodes::GprNumber::X8, @@ -1065,7 +1066,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec Opcodes::Ebreak, }); - if (programOpcodes.size() > this->debugModuleDescriptor.programBufferSize) { + if (this->debugModuleDescriptor.programBufferSize < PROGRAM_OPCODES.size()) { throw Exceptions::TargetOperationFailure{ "Cannot read memory via RISC-V debug module program buffer - insufficient program buffer size" }; @@ -1075,7 +1076,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec auto preservedX9Register = PreservedCpuRegister{CpuRegisterNumber::GPR_X9, *this}; try { - this->writeProgramBuffer(programOpcodes); + this->writeProgramBuffer(PROGRAM_OPCODES); auto commandError = this->tryWriteCpuRegister( CpuRegisterNumber::GPR_X8, @@ -1188,7 +1189,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec assert(startAddress % DebugTranslator::WORD_BYTE_SIZE == 0); assert(buffer.size() % DebugTranslator::WORD_BYTE_SIZE == 0); - static constexpr auto programOpcodes = std::to_array({ + static constexpr auto PROGRAM_OPCODES = std::to_array({ Opcodes::Sw{ .baseAddressRegister = Opcodes::GprNumber::X8, .valueRegister = Opcodes::GprNumber::X9, @@ -1202,7 +1203,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec Opcodes::Ebreak, }); - if (programOpcodes.size() > this->debugModuleDescriptor.programBufferSize) { + if (this->debugModuleDescriptor.programBufferSize < PROGRAM_OPCODES.size()) { throw Exceptions::TargetOperationFailure{ "Cannot write to memory via RISC-V debug module program buffer - insufficient program buffer size" }; @@ -1212,7 +1213,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec auto preservedX9Register = PreservedCpuRegister{CpuRegisterNumber::GPR_X9, *this}; try { - this->writeProgramBuffer(programOpcodes); + this->writeProgramBuffer(PROGRAM_OPCODES); this->writeCpuRegister(CpuRegisterNumber::GPR_X8, startAddress, {.postExecute = false}); this->writeCpuRegister( @@ -1347,7 +1348,7 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec } catch (const Exceptions::Exception& exception) { /* - * If we fail to restore the value of a CPU register, we must escalate this to a fatal error, as the target + * If we fail to restore a preserved CPU register, we must escalate this to a fatal error, as the target * will be left in an undefined state. More specifically, the state of the program running on the target * may be corrupted. We cannot recover from this. * diff --git a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp index a736f039..4e5d746e 100644 --- a/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp +++ b/src/DebugToolDrivers/Protocols/RiscVDebugSpec/DebugTranslator.hpp @@ -187,12 +187,6 @@ namespace DebugToolDrivers::Protocols::RiscVDebugSpec DebugModule::MemoryAccessStrategy determineMemoryAccessStrategy(); - Targets::TargetMemoryAddress alignMemoryAddress( - Targets::TargetMemoryAddress address, - Targets::TargetMemoryAddress alignTo - ); - Targets::TargetMemorySize alignMemorySize(Targets::TargetMemorySize size, Targets::TargetMemorySize alignTo); - Targets::TargetMemoryBuffer readMemoryViaAbstractCommand( Targets::TargetMemoryAddress startAddress, Targets::TargetMemorySize bytes diff --git a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp index 6086adb0..3184ca6f 100644 --- a/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp +++ b/src/DebugToolDrivers/Wch/WchLinkDebugInterface.hpp @@ -21,10 +21,10 @@ namespace DebugToolDrivers::Wch { /** - * The RISC-V debug module cannot provide a complete implementation of the RiscVDebugInterface. + * The RISC-V debug translator cannot provide a complete implementation of the RiscVDebugInterface. * - * This class combines the functionality of the RISC-V debug module (via the RiscVDebugTranslator), with the - * WCH-Link-specific functionality, to provide a complete implementation. + * This class combines the functionality of the debug translator, with the WCH-Link-specific functionality, to + * provide a complete implementation. */ class WchLinkDebugInterface : public TargetInterfaces::RiscV::RiscVDebugInterface