doryan/BloomPatched
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Lots of tidying

Browse Source 
- Removed generic `avr8` target
- Simplified AVR8 target construction
- Introduced register descriptor IDs
- Simplified GDB register mappings
- Simplified target interface contract
- Other bits of tidying
This commit is contained in:
Nav
2023-05-21 21:08:25 +01:00
parent 5f8242a87a
commit ba03833325
62 changed files with 1304 additions and 1577 deletions
Download Patch File Download Diff File Expand all files Collapse all files

235
src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp
View File

@@ -1,6 +1,7 @@
#include "EdbgAvr8Interface.hpp"
#include <thread>
#include <cassert>
#include <cmath>
#include "src/Services/PathService.hpp"
@@ -80,71 +81,26 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
using Bloom::Targets::TargetRegister;
using Bloom::Targets::TargetRegisterDescriptor;
using Bloom::Targets::TargetRegisterDescriptors;
using Bloom::Targets::TargetRegisterDescriptorId;
using Bloom::Targets::TargetRegisterDescriptorIds;
using Bloom::Targets::TargetRegisterType;
using Bloom::Targets::TargetRegisters;
EdbgAvr8Interface::EdbgAvr8Interface(EdbgInterface* edbgInterface)
EdbgAvr8Interface::EdbgAvr8Interface(
EdbgInterface* edbgInterface,
const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig,
Targets::Microchip::Avr::Avr8Bit::Family targetFamily,
const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters,
const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById
)
: edbgInterface(edbgInterface)
, targetConfig(targetConfig)
, family(targetFamily)
, targetParameters(targetParameters)
, targetRegisterDescriptorsById(targetRegisterDescriptorsById)
, configVariant(EdbgAvr8Interface::resolveConfigVariant(targetFamily, targetConfig.physicalInterface))
{}
void EdbgAvr8Interface::configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) {
this->targetConfig = targetConfig;
this->configVariant = this->resolveConfigVariant().value_or(Avr8ConfigVariant::NONE);
}
void EdbgAvr8Interface::setTargetParameters(const Avr8Bit::TargetParameters& config) {
this->targetParameters = config;
if (!config.stackPointerRegisterLowAddress.has_value()) {
throw DeviceInitializationFailure("Failed to find stack pointer register start address");
}
if (!config.stackPointerRegisterSize.has_value()) {
throw DeviceInitializationFailure("Failed to find stack pointer register size");
}
if (!config.statusRegisterStartAddress.has_value()) {
throw DeviceInitializationFailure("Failed to find status register start address");
}
if (!config.statusRegisterSize.has_value()) {
throw DeviceInitializationFailure("Failed to find status register size");
}
if (this->configVariant == Avr8ConfigVariant::NONE) {
auto configVariant = this->resolveConfigVariant();
if (!configVariant.has_value()) {
throw DeviceInitializationFailure("Failed to resolve config variant for the selected "
"physical interface and AVR8 family. The selected physical interface is not known to be supported "
"by the AVR8 family."
);
}
this->configVariant = configVariant.value();
}
switch (this->configVariant) {
case Avr8ConfigVariant::DEBUG_WIRE:
case Avr8ConfigVariant::MEGAJTAG: {
this->setDebugWireAndJtagParameters();
break;
}
case Avr8ConfigVariant::XMEGA: {
this->setPdiParameters();
break;
}
case Avr8ConfigVariant::UPDI: {
this->setUpdiParameters();
break;
}
default: {
break;
}
}
}
void EdbgAvr8Interface::init() {
if (this->configVariant == Avr8ConfigVariant::XMEGA) {
// Default PDI clock to 4MHz
@@ -177,8 +133,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
this->setParameter(
Avr8EdbgParameters::PHYSICAL_INTERFACE,
getAvr8PhysicalInterfaceToIdMapping().at(this->targetConfig->physicalInterface)
getAvr8PhysicalInterfaceToIdMapping().at(this->targetConfig.physicalInterface)
);
this->setTargetParameters();
}
void EdbgAvr8Interface::stop() {
@@ -267,7 +225,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
} catch (const Avr8CommandFailure& activationException) {
if (
this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE
this->targetConfig.physicalInterface == PhysicalInterface::DEBUG_WIRE
&& (
activationException.code == Avr8CommandFailureCode::DEBUGWIRE_PHYSICAL_ERROR
|| activationException.code == Avr8CommandFailureCode::FAILED_TO_ENABLE_OCD
@@ -292,8 +250,8 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
void EdbgAvr8Interface::deactivate() {
if (this->targetAttached) {
if (
this->targetConfig->physicalInterface == PhysicalInterface::DEBUG_WIRE
&& this->targetConfig->disableDebugWireOnDeactivate
this->targetConfig.physicalInterface == PhysicalInterface::DEBUG_WIRE
&& this->targetConfig.disableDebugWireOnDeactivate
) {
try {
this->disableDebugWire();
@@ -385,7 +343,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
throw Avr8CommandFailure("AVR8 Get device ID command failed", responseFrame);
}
return responseFrame.extractSignature(this->targetConfig->physicalInterface);
return responseFrame.extractSignature(this->targetConfig.physicalInterface);
}
void EdbgAvr8Interface::setBreakpoint(TargetMemoryAddress address) {
@@ -418,7 +376,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
}
TargetRegisters EdbgAvr8Interface::readRegisters(const TargetRegisterDescriptors& descriptors) {
TargetRegisters EdbgAvr8Interface::readRegisters(const TargetRegisterDescriptorIds& descriptorIds) {
/*
* This function needs to be fast. Insight eagerly requests the values of all known registers that it can
* present to the user. It does this on numerous occasions (target stopped, user clicked refresh, etc). This
@@ -439,7 +397,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
auto output = TargetRegisters();
// Group descriptors by type and resolve the address range for each type
auto descriptorsByType = std::map<TargetRegisterType, std::set<const TargetRegisterDescriptor*>>();
auto descriptorIdsByType = std::map<TargetRegisterType, std::set<TargetRegisterDescriptorId>>();
/*
* An address range is just an std::pair of addresses - the first being the start address, the second being
@@ -450,7 +408,12 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
using AddressRange = std::pair<TargetMemoryAddress, TargetMemoryAddress>;
auto addressRangeByType = std::map<TargetRegisterType, AddressRange>();
for (const auto& descriptor : descriptors) {
for (const auto& descriptorId : descriptorIds) {
const auto descriptorIt = this->targetRegisterDescriptorsById.find(descriptorId);
assert(descriptorIt != this->targetRegisterDescriptorsById.end());
const auto& descriptor = descriptorIt->second;
if (!descriptor.startAddress.has_value()) {
Logger::debug(
"Attempted to read register in the absence of a start address - register name: "
@@ -459,18 +422,18 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
continue;
}
descriptorsByType[descriptor.type].insert(&descriptor);
descriptorIdsByType[descriptor.type].insert(descriptor.id);
const auto startAddress = descriptor.startAddress.value();
const auto endAddress = startAddress + (descriptor.size - 1);
const auto addressRangeit = addressRangeByType.find(descriptor.type);
const auto addressRangeIt = addressRangeByType.find(descriptor.type);
if (addressRangeit == addressRangeByType.end()) {
if (addressRangeIt == addressRangeByType.end()) {
addressRangeByType[descriptor.type] = AddressRange(startAddress, endAddress);
} else {
auto& addressRange = addressRangeit->second;
auto& addressRange = addressRangeIt->second;
if (startAddress < addressRange.first) {
addressRange.first = startAddress;
@@ -486,16 +449,17 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
* Now that we have our address ranges and grouped descriptors, we can perform a single read call for each
* register type.
*/
for (const auto&[registerType, descriptors] : descriptorsByType) {
for (const auto&[registerType, descriptorIds] : descriptorIdsByType) {
const auto& addressRange = addressRangeByType[registerType];
const auto startAddress = addressRange.first;
const auto endAddress = addressRange.second;
const auto bufferSize = (endAddress - startAddress) + 1;
const auto readSize = (endAddress - startAddress) + 1;
const auto memoryType = (registerType != TargetRegisterType::GENERAL_PURPOSE_REGISTER) ?
Avr8MemoryType::SRAM
const auto memoryType = (registerType != TargetRegisterType::GENERAL_PURPOSE_REGISTER)
? Avr8MemoryType::SRAM
: (this->configVariant == Avr8ConfigVariant::XMEGA || this->configVariant == Avr8ConfigVariant::UPDI
? Avr8MemoryType::REGISTER_FILE : Avr8MemoryType::SRAM);
? Avr8MemoryType::REGISTER_FILE
: Avr8MemoryType::SRAM);
/*
* When reading the entire range, we must avoid any attempts to access the OCD data register (OCDDR), as
@@ -526,37 +490,40 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
);
}
const auto flatMemoryBuffer = this->readMemory(
const auto flatMemoryData = this->readMemory(
memoryType,
startAddress,
bufferSize,
readSize,
excludedAddresses
);
if (flatMemoryBuffer.size() != bufferSize) {
if (flatMemoryData.size() != readSize) {
throw Exception(
"Failed to read memory within register type address range (" + std::to_string(startAddress)
+ " - " + std::to_string(endAddress) + "). Expected " + std::to_string(bufferSize)
+ " bytes, got " + std::to_string(flatMemoryBuffer.size())
+ " - " + std::to_string(endAddress) + "). Expected " + std::to_string(readSize)
+ " bytes, got " + std::to_string(flatMemoryData.size())
);
}
// Construct our TargetRegister objects directly from the flat memory buffer
for (const auto& descriptor : descriptors) {
for (const auto descriptorId : descriptorIds) {
const auto descriptorIt = this->targetRegisterDescriptorsById.find(descriptorId);
const auto& descriptor = descriptorIt->second;
/*
* Multibyte AVR8 registers are stored in LSB form.
*
* This is why we use reverse iterators when extracting our data from flatMemoryBuffer. Doing so allows
* This is why we use reverse iterators when extracting our data from flatMemoryData. Doing so allows
* us to extract the data in MSB form (as is expected for all register values held in TargetRegister
* objects).
*/
const auto bufferStartIt = flatMemoryBuffer.rend() - (descriptor->startAddress.value() - startAddress)
- descriptor->size;
const auto bufferStartIt = flatMemoryData.rend() - (descriptor.startAddress.value() - startAddress)
- descriptor.size;
output.emplace_back(
TargetRegister(
*descriptor,
TargetMemoryBuffer(bufferStartIt, bufferStartIt + descriptor->size)
descriptor.id,
TargetMemoryBuffer(bufferStartIt, bufferStartIt + descriptor.size)
)
);
}
@@ -567,7 +534,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
void EdbgAvr8Interface::writeRegisters(const Targets::TargetRegisters& registers) {
for (const auto& reg : registers) {
const auto& registerDescriptor = reg.descriptor;
const auto& registerDescriptorIt = this->targetRegisterDescriptorsById.find(reg.descriptorId);
assert(registerDescriptorIt != this->targetRegisterDescriptorsById.end());
const auto& registerDescriptor = registerDescriptorIt->second;
auto registerValue = reg.value;
if (registerValue.empty()) {
@@ -827,7 +797,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
*/
auto eepromSnapshot = std::optional<Targets::TargetMemoryBuffer>();
if (this->targetConfig->preserveEeprom) {
if (this->targetConfig.preserveEeprom) {
Logger::debug("Capturing EEPROM data, in preparation for chip erase");
eepromSnapshot = this->readMemory(
TargetMemoryType::EEPROM,
@@ -910,6 +880,43 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
}
}
void EdbgAvr8Interface::setTargetParameters() {
if (!this->targetParameters.stackPointerRegisterLowAddress.has_value()) {
throw DeviceInitializationFailure("Failed to find stack pointer register start address");
}
if (!this->targetParameters.stackPointerRegisterSize.has_value()) {
throw DeviceInitializationFailure("Failed to find stack pointer register size");
}
if (!this->targetParameters.statusRegisterStartAddress.has_value()) {
throw DeviceInitializationFailure("Failed to find status register start address");
}
if (!this->targetParameters.statusRegisterSize.has_value()) {
throw DeviceInitializationFailure("Failed to find status register size");
}
switch (this->configVariant) {
case Avr8ConfigVariant::DEBUG_WIRE:
case Avr8ConfigVariant::MEGAJTAG: {
this->setDebugWireAndJtagParameters();
break;
}
case Avr8ConfigVariant::XMEGA: {
this->setPdiParameters();
break;
}
case Avr8ConfigVariant::UPDI: {
this->setUpdiParameters();
break;
}
default: {
break;
}
}
}
std::map<Family, std::map<PhysicalInterface, Avr8ConfigVariant>>
EdbgAvr8Interface::getConfigVariantsByFamilyAndPhysicalInterface() {
return std::map<Family, std::map<PhysicalInterface, Avr8ConfigVariant>>({
@@ -963,49 +970,21 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
});
}
std::optional<Avr8ConfigVariant> EdbgAvr8Interface::resolveConfigVariant() {
if (this->family.has_value()) {
const auto configVariantsByFamily = EdbgAvr8Interface::getConfigVariantsByFamilyAndPhysicalInterface();
const auto configVariantsByPhysicalInterfaceIt = configVariantsByFamily.find(*(this->family));
Avr8ConfigVariant EdbgAvr8Interface::resolveConfigVariant(
Targets::Microchip::Avr::Avr8Bit::Family targetFamily,
Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface
) {
const auto configVariantsByFamily = EdbgAvr8Interface::getConfigVariantsByFamilyAndPhysicalInterface();
const auto configVariantsByPhysicalInterfaceIt = configVariantsByFamily.find(targetFamily);
if (configVariantsByPhysicalInterfaceIt != configVariantsByFamily.end()) {
const auto& configVariantsByPhysicalInterface = configVariantsByPhysicalInterfaceIt->second;
const auto configVariantIt = configVariantsByPhysicalInterface.find(
this->targetConfig->physicalInterface
);
assert(configVariantsByPhysicalInterfaceIt != configVariantsByFamily.end());
if (configVariantIt != configVariantsByPhysicalInterface.end()) {
return configVariantIt->second;
}
}
const auto& configVariantsByPhysicalInterface = configVariantsByPhysicalInterfaceIt->second;
const auto configVariantIt = configVariantsByPhysicalInterface.find(physicalInterface);
} else {
/*
* If there is no family set, we may be able to resort to a simpler mapping of physical interfaces
* to config variants. But this will only work if the selected physical interface is *NOT* JTAG.
*
* This is because JTAG is the only physical interface that could map to two different config
* variants (MEGAJTAG and XMEGA). The only way we can figure out which config variant to use is if we
* know the target family.
*
* This is why we don't allow users to use ambiguous target names (such as the generic "avr8" target
* name), when using the JTAG physical interface. We won't be able to resolve the correct target
* variant. Users are required to specify the exact target name in their config, when using the JTAG
* physical interface. That way, this->family will be set by the time resolveConfigVariant() is called.
*/
static const std::map<PhysicalInterface, Avr8ConfigVariant> physicalInterfacesToConfigVariants = {
{PhysicalInterface::DEBUG_WIRE, Avr8ConfigVariant::DEBUG_WIRE},
{PhysicalInterface::PDI, Avr8ConfigVariant::XMEGA},
{PhysicalInterface::UPDI, Avr8ConfigVariant::UPDI},
};
const auto configVariantIt = physicalInterfacesToConfigVariants.find(this->targetConfig->physicalInterface);
assert(configVariantIt != configVariantsByPhysicalInterface.end());
if (configVariantIt != physicalInterfacesToConfigVariants.end()) {
return configVariantIt->second;
}
}
return std::nullopt;
return configVariantIt->second;
}
void EdbgAvr8Interface::setParameter(const Avr8EdbgParameter& parameter, const std::vector<unsigned char>& value) {

63
src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp
View File

@@ -10,9 +10,10 @@
#include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Avr8Generic.hpp"
#include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/EdbgInterface.hpp"
#include "src/Targets/TargetMemory.hpp"
#include "src/Targets/Microchip/AVR/Target.hpp"
#include "src/Targets/TargetRegister.hpp"
#include "src/Targets/Microchip/AVR/AVR8/Family.hpp"
#include "src/Targets/Microchip/AVR/AVR8/PhysicalInterface.hpp"
#include "src/Targets/Microchip/AVR/AVR8/TargetParameters.hpp"
namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
{
@@ -28,7 +29,13 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
class EdbgAvr8Interface: public TargetInterfaces::Microchip::Avr::Avr8::Avr8DebugInterface
{
public:
explicit EdbgAvr8Interface(EdbgInterface* edbgInterface);
explicit EdbgAvr8Interface(
EdbgInterface* edbgInterface,
const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig,
Targets::Microchip::Avr::Avr8Bit::Family targetFamily,
const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters,
const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById
);
/**
* Some EDBG devices don't seem to operate correctly when actioning the masked memory read EDBG command. The
@@ -83,32 +90,6 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
* See the comments in that class for more info on the expected behaviour of each method.
*/
/**
* As already mentioned in numerous comments above, the EdbgAvr8Interface requires some configuration from
* the user. This is supplied via the user's Bloom configuration.
*
* @param targetConfig
*/
void configure(const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig) override;
/**
* Configures the target family. For some physical interfaces, the target family is required in order
* properly configure the EDBG tool. See EdbgAvr8Interface::resolveConfigVariant() for more.
*
* @param family
*/
void setFamily(Targets::Microchip::Avr::Avr8Bit::Family family) override {
this->family = family;
}
/**
* Accepts target parameters from the AVR8 target instance and sends the necessary target parameters to the
* debug tool.
*
* @param config
*/
void setTargetParameters(const Targets::Microchip::Avr::Avr8Bit::TargetParameters& config) override;
/**
* Initialises the AVR8 Generic protocol interface by setting the appropriate parameters on the debug tool.
*/
@@ -207,10 +188,10 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
/**
* Reads registers from the target.
*
* @param descriptors
* @param descriptorIds
* @return
*/
Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptors& descriptors) override;
Targets::TargetRegisters readRegisters(const Targets::TargetRegisterDescriptorIds& descriptorIds) override;
/**
* Writes registers to target.
@@ -289,7 +270,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
/**
* Project's AVR8 target configuration.
*/
std::optional<Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig> targetConfig;
const Targets::Microchip::Avr::Avr8Bit::Avr8TargetConfig& targetConfig;
/**
* The target family is taken into account when configuring the AVR8 Generic protocol on the EDBG device.
@@ -297,7 +278,7 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
* We use this to determine which config variant to select.
* See EdbgAvr8Interface::resolveConfigVariant() for more.
*/
std::optional<Targets::Microchip::Avr::Avr8Bit::Family> family;
Targets::Microchip::Avr::Avr8Bit::Family family;
/**
* The AVR8 Generic protocol provides two functions: Debugging and programming. The desired function must be
@@ -320,7 +301,9 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
* For the EdbgAvr8Interface, we send the required parameters to the debug tool immediately upon receiving
* them. See EdbgAvr8Interface::setTargetParameters().
*/
Targets::Microchip::Avr::Avr8Bit::TargetParameters targetParameters;
const Targets::Microchip::Avr::Avr8Bit::TargetParameters& targetParameters;
const Targets::TargetRegisterDescriptorMapping& targetRegisterDescriptorsById;
/**
* See the comment for EdbgAvr8Interface::setAvoidMaskedMemoryRead().
@@ -357,6 +340,13 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
bool programmingModeEnabled = false;
/**
* Sends the necessary target parameters to the debug tool.
*
* @param config
*/
void setTargetParameters();
/**
* This mapping allows us to determine which config variant to select, based on the target family and the
* selected physical interface.
@@ -367,11 +357,14 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
> getConfigVariantsByFamilyAndPhysicalInterface();
/**
* Will attempt to resolve the config variant with the information currently held.
* Determines the config variant given a target family and physical interface.
*
* @return
*/
std::optional<Avr8ConfigVariant> resolveConfigVariant();
static Avr8ConfigVariant resolveConfigVariant(
Targets::Microchip::Avr::Avr8Bit::Family targetFamily,
Targets::Microchip::Avr::Avr8Bit::PhysicalInterface physicalInterface
);
/**
* Sets an AVR8 parameter on the debug tool. See the Avr8EdbgParameters class and protocol documentation

4
src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/Events/AVR8Generic/BreakEvent.hpp
View File

@@ -2,8 +2,8 @@
#include <cstdint>
#include "../../AvrEvent.hpp"
#include "src/Targets/Microchip/AVR/Target.hpp"
#include "src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/AvrEvent.hpp"
#include "src/Targets/TargetBreakpoint.hpp"
namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
{