From ee20507c2ea7154283a7c771ae1b2f3b6b651ce6 Mon Sep 17 00:00:00 2001 From: Nav Date: Sun, 2 Jun 2024 21:29:57 +0100 Subject: [PATCH] Corrected debugWire and JTAG HIGH byte parameter values Additional checks in TDF validation to ensure that the IO memory segment offset has been applied to the relevant registers --- .../AVR8/Avr8TargetDescriptionFile.php | 50 ++++++++----- .../AVR8/Services/ValidationService.php | 75 +++++++++++++++++-- .../AVR8Generic/DebugWireJtagParameters.cpp | 27 ++++--- 3 files changed, 117 insertions(+), 35 deletions(-) diff --git a/build/scripts/Targets/TargetDescriptionFiles/AVR8/Avr8TargetDescriptionFile.php b/build/scripts/Targets/TargetDescriptionFiles/AVR8/Avr8TargetDescriptionFile.php index fcdf2195..f4e3d685 100644 --- a/build/scripts/Targets/TargetDescriptionFiles/AVR8/Avr8TargetDescriptionFile.php +++ b/build/scripts/Targets/TargetDescriptionFiles/AVR8/Avr8TargetDescriptionFile.php @@ -132,12 +132,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile if ($eepromAddressRegister instanceof TargetRegister) { $output->eearAddressLow = $eepromAddressRegister->address; - $output->eearAddressHigh = $eepromAddressRegister->size > 1 - ? $eepromAddressRegister->address !== null - ? $eepromAddressRegister->address >> 8 - : null - : $eepromAddressRegister->address - ; + $output->eearAddressHigh = $eepromAddressRegister->address + ($eepromAddressRegister->size - 1); } else { $eearAddressLow = $eepromPeripheral->getRegister('eeprom', 'eearl'); @@ -145,11 +140,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile if ($eearAddressLow instanceof TargetRegister) { $output->eearAddressLow = $eearAddressLow->address; - $output->eearAddressHigh = $eearAddressLow->size > 1 - ? $eearAddressLow->address !== null - ? $eearAddressLow->address >> 8 - : null - : $eearAddressLow->address; + $output->eearAddressHigh = $eearAddressLow->address + ($eearAddressLow->size - 1); } if ($eearAddressHigh instanceof TargetRegister) { @@ -283,12 +274,7 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile if ($eepromAddressRegister instanceof TargetRegister) { $output->eearAddressLow = $eepromAddressRegister->address; - $output->eearAddressHigh = $eepromAddressRegister->size > 1 - ? $eepromAddressRegister->address !== null - ? $eepromAddressRegister->address >> 2 - : null - : $eepromAddressRegister->address - ; + $output->eearAddressHigh = $eepromAddressRegister->address + ($eepromAddressRegister->size - 1); } else { $eearAddressLow = $eepromPeripheral->getRegister('eeprom', 'eearl'); @@ -509,22 +495,46 @@ class Avr8TargetDescriptionFile extends TargetDescriptionFile return null; } - private function getProgramMemorySegment(): ?MemorySegment + public function getProgramMemorySegment(): ?MemorySegment { return $this->getMemorySegment('prog', 'internal_program_memory'); } - private function getRamSegment(): ?MemorySegment + public function getGpRegistersMemorySegment(): ?MemorySegment + { + return $this->getMemorySegment('data', 'gp_registers') + ?? $this->getMemorySegment('register_file', 'gp_registers'); + } + + public function getRamSegment(): ?MemorySegment { return $this->getMemorySegment('data', 'internal_ram'); } - private function getEepromSegment(): ?MemorySegment + public function getEepromSegment(): ?MemorySegment { return $this->getMemorySegment('eeprom', 'internal_eeprom') ?? $this->getMemorySegment('data', 'internal_eeprom'); } + public function getIoMemorySegment(): ?MemorySegment + { + return $this->getMemorySegment('data', 'io') + ?? $this->getMemorySegment('data', 'mapped_io'); + } + + public function getSignaturesMemorySegment(): ?MemorySegment + { + return $this->getMemorySegment('data', 'signatures') + ?? $this->getMemorySegment('signatures', 'signatures'); + } + + public function getFusesMemorySegment(): ?MemorySegment + { + return $this->getMemorySegment('data', 'fuses') + ?? $this->getMemorySegment('fuses', 'fuses'); + } + private function getBootSectionOptions(): array { $output = []; diff --git a/build/scripts/Targets/TargetDescriptionFiles/AVR8/Services/ValidationService.php b/build/scripts/Targets/TargetDescriptionFiles/AVR8/Services/ValidationService.php index ba35fcab..427b5708 100644 --- a/build/scripts/Targets/TargetDescriptionFiles/AVR8/Services/ValidationService.php +++ b/build/scripts/Targets/TargetDescriptionFiles/AVR8/Services/ValidationService.php @@ -9,6 +9,7 @@ use Targets\TargetDescriptionFiles\Avr8\IspParameters; use Targets\TargetDescriptionFiles\Avr8\JtagParameters; use Targets\TargetDescriptionFiles\Avr8\PdiParameters; use Targets\TargetDescriptionFiles\Avr8\UpdiParameters; +use Targets\TargetDescriptionFiles\MemorySegment; use Targets\TargetRegister; use Targets\TargetRegisterGroup; @@ -137,7 +138,11 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat } if (in_array(AvrPhysicalInterface::DEBUG_WIRE, $debugPhysicalInterfaces)) { - $failures = array_merge($failures, $this->validateDebugWireParameters($tdf->getDebugWireParameters())); + $failures = array_merge( + $failures, + $this->validateDebugWireParameters($tdf->getDebugWireParameters(), $tdf) + ); + $failures = array_merge($failures, $this->validateIspParameters($tdf->getIspParameters())); if (!in_array(AvrPhysicalInterface::ISP, $physicalInterfaces)) { @@ -165,7 +170,7 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat in_array(AvrPhysicalInterface::JTAG, $debugPhysicalInterfaces) && $family == AvrFamily::MEGA ) { - $failures = array_merge($failures, $this->validateJtagParameters($tdf->getJtagParameters())); + $failures = array_merge($failures, $this->validateJtagParameters($tdf->getJtagParameters(), $tdf)); if (empty($tdf->getFuseBitsDescriptor('ocden'))) { $failures[] = 'Could not find OCDEN fuse bit field for JTAG target'; @@ -233,8 +238,10 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat return $failures; } - private function validateDebugWireParameters(DebugWireParameters $parameters): array - { + private function validateDebugWireParameters( + DebugWireParameters $parameters, + Avr8TargetDescriptionFile $tdf + ): array { $failures = []; if ($parameters->flashPageSize === null) { @@ -336,6 +343,35 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat $failures[] = 'OSCCALR address size exceeds 0xFF - corresponding EDBG device parameter size is 8 bits'; } + /* + * Bloom removes the IO memory segment offset when sending some of these params to the debug tool. This means + * we assume that the offset has already been applied to the params. We enforce this here. + */ + if (($ioMemorySegment = $tdf->getIoMemorySegment()) instanceof MemorySegment) { + if ($parameters->osccalAddress < $ioMemorySegment->startAddress) { + $failures[] = 'OSCCAL address does not have IO memory segment offset applied'; + } + + if ($parameters->eearAddressLow < $ioMemorySegment->startAddress) { + $failures[] = 'EEARL address does not have IO memory segment offset applied'; + } + + if ($parameters->eearAddressHigh < $ioMemorySegment->startAddress) { + $failures[] = 'EEARH address does not have IO memory segment offset applied'; + } + + if ($parameters->eecrAddress < $ioMemorySegment->startAddress) { + $failures[] = 'EECR address does not have IO memory segment offset applied'; + } + + if ($parameters->eedrAddress < $ioMemorySegment->startAddress) { + $failures[] = 'EEDR address does not have IO memory segment offset applied'; + } + + } else { + $failures[] = 'Could not verify address offset of debugWire parameters - IO memory segment missing'; + } + return $failures; } @@ -394,7 +430,7 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat return $failures; } - private function validateJtagParameters(JtagParameters $parameters): array + private function validateJtagParameters(JtagParameters $parameters, Avr8TargetDescriptionFile $tdf): array { $failures = []; @@ -497,6 +533,35 @@ class ValidationService extends \Targets\TargetDescriptionFiles\Services\Validat $failures[] = 'OSCCALR address size exceeds 0xFF - corresponding EDBG device parameter size is 8 bits'; } + /* + * Bloom removes the IO memory segment offset when sending some of these params to the debug tool. This means + * we assume that the offset has already been applied to the params. We enforce this here. + */ + if (($ioMemorySegment = $tdf->getIoMemorySegment()) instanceof MemorySegment) { + if ($parameters->osccalAddress < $ioMemorySegment->startAddress) { + $failures[] = 'OSCCAL address does not have IO memory segment offset applied'; + } + + if ($parameters->eearAddressLow < $ioMemorySegment->startAddress) { + $failures[] = 'EEARL address does not have IO memory segment offset applied'; + } + + if ($parameters->eearAddressHigh < $ioMemorySegment->startAddress) { + $failures[] = 'EEARH address does not have IO memory segment offset applied'; + } + + if ($parameters->eecrAddress < $ioMemorySegment->startAddress) { + $failures[] = 'EECR address does not have IO memory segment offset applied'; + } + + if ($parameters->eedrAddress < $ioMemorySegment->startAddress) { + $failures[] = 'EEDR address does not have IO memory segment offset applied'; + } + + } else { + $failures[] = 'Could not verify address offset of debugWire parameters - IO memory segment missing'; + } + return $failures; } diff --git a/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/Parameters/AVR8Generic/DebugWireJtagParameters.cpp b/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/Parameters/AVR8Generic/DebugWireJtagParameters.cpp index 75fee3c6..b76fed4b 100644 --- a/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/Parameters/AVR8Generic/DebugWireJtagParameters.cpp +++ b/src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/Parameters/AVR8Generic/DebugWireJtagParameters.cpp @@ -48,9 +48,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr::Parameters::Avr8Gen * If the target doesn't have a high byte in the `EEAR` address, the `eearAddressHigh` parameter should be * equal to the `eearAddressLow` parameter, as stated in the "EDBG-based Tools Protocols" document. */ - this->eearAddressHigh = static_cast( - eearDescriptor->get().size > 1 ? startAddress >> 8 : startAddress - ); + this->eearAddressHigh = static_cast(startAddress + (eearDescriptor->get().size - 1)); } else { const auto& eearlDescriptor = eepromRegisterGroupDescriptor.getRegisterDescriptor("eearl"); @@ -67,7 +65,7 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr::Parameters::Avr8Gen this->eearAddressHigh = static_cast( eearhDescriptor.has_value() ? eearhDescriptor->get().startAddress - : eearlDescriptor.size > 1 ? eearlDescriptor.startAddress >> 8 : eearlDescriptor.startAddress + : eearlDescriptor.startAddress + (eearlDescriptor.size - 1) ); } @@ -130,12 +128,21 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr::Parameters::Avr8Gen * * It does *not* seem to apply to the SPMCR or OCDDR address. */ - const auto& ioMemorySegment = targetDescriptionFile.getIoMemorySegment(); + const auto& ioSegmentStartAddress = static_cast( + targetDescriptionFile.getIoMemorySegment().startAddress + ); - this->osccalAddress -= ioMemorySegment.startAddress; - this->eearAddressLow -= ioMemorySegment.startAddress; - this->eearAddressHigh -= ioMemorySegment.startAddress; - this->eecrAddress -= ioMemorySegment.startAddress; - this->eedrAddress -= ioMemorySegment.startAddress; + // This is enforced in TDF validation + assert(this->osccalAddress >= ioSegmentStartAddress); + assert(this->eearAddressLow >= ioSegmentStartAddress); + assert(this->eearAddressHigh >= ioSegmentStartAddress); + assert(this->eecrAddress >= ioSegmentStartAddress); + assert(this->eedrAddress >= ioSegmentStartAddress); + + this->osccalAddress -= ioSegmentStartAddress; + this->eearAddressLow -= ioSegmentStartAddress; + this->eearAddressHigh -= ioSegmentStartAddress; + this->eecrAddress -= ioSegmentStartAddress; + this->eedrAddress -= ioSegmentStartAddress; } }