From f2025020050829450c940ce619c49b547932fe63 Mon Sep 17 00:00:00 2001
From: Nav <nav@navnav.me>
Date: Mon, 22 Nov 2021 23:07:18 +0000
Subject: [PATCH] Added maximumMemoryAccessSizePerRequest limit in
 EdbgAvr8Interface class to allow debug tools to apply a hard limit on memory
 access sizes per request

---
 .../EDBG/AVR/EdbgAvr8Interface.cpp            | 43 ++++++++++++++++---
 .../EDBG/AVR/EdbgAvr8Interface.hpp            | 22 ++++++++++
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp
index 7c18c15b..22ba34f7 100644
--- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp
+++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.cpp
@@ -1220,13 +1220,18 @@ TargetMemoryBuffer EdbgAvr8Interface::readMemory(
     }
 
     if (type == Avr8MemoryType::FLASH_PAGE) {
-        if (this->targetParameters.flashPageSize.value_or(0) < 1) {
+        // Flash reads must be done in pages
+        auto pageSize = this->targetParameters.flashPageSize.value_or(0);
+
+        if (pageSize < 1
+            || (
+                this->maximumMemoryAccessSizePerRequest.has_value()
+                && pageSize > this->maximumMemoryAccessSizePerRequest
+            )
+        ) {
             throw Exception("Missing/invalid flash page size parameter");
         }
 
-        // Flash reads must be done in pages
-        auto pageSize = this->targetParameters.flashPageSize.value();
-
         if ((bytes % pageSize) != 0 || (startAddress % pageSize) != 0) {
             /*
              * The number of bytes to read and/or the start address are not aligned.
@@ -1316,7 +1321,35 @@ TargetMemoryBuffer EdbgAvr8Interface::readMemory(
             return memoryBuffer;
         }
 
-    } else {
+    }
+
+    /*
+     * Enforce a maximum memory access request size.
+     *
+     * See the comment for EdbgAvr8Interface::setMaximumMemoryAccessSizePerRequest() for more on this.
+     */
+    if (this->maximumMemoryAccessSizePerRequest.has_value() && bytes > this->maximumMemoryAccessSizePerRequest) {
+        auto maximumRequestSize = this->maximumMemoryAccessSizePerRequest.value();
+        auto totalReadsRequired = std::ceil(static_cast<float>(bytes) / static_cast<float>(maximumRequestSize));
+        auto output = std::vector<unsigned char>();
+        output.reserve(bytes);
+
+        for (float i = 1; i <= totalReadsRequired; i++) {
+            auto bytesToRead = static_cast<std::uint32_t>((bytes - output.size()) > maximumRequestSize ?
+                maximumRequestSize : bytes - output.size());
+            auto data = this->readMemory(
+                type,
+                static_cast<std::uint32_t>(startAddress + output.size()),
+                bytesToRead,
+                excludedAddresses
+            );
+            output.insert(output.end(), data.begin(), data.end());
+        }
+
+        return output;
+    }
+
+    if (type != Avr8MemoryType::FLASH_PAGE) {
         /*
          * EDBG AVR8 debug tools behave in a really weird way when responding with more than two packets
          * for a single read (non-flash) memory command. The data they return in this case appears to be of little use.
diff --git a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp
index 16e6faab..ed87efcb 100644
--- a/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp
+++ b/src/DebugToolDrivers/Protocols/CMSIS-DAP/VendorSpecific/EDBG/AVR/EdbgAvr8Interface.hpp
@@ -38,6 +38,26 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
             this->avoidMaskedMemoryRead = avoidMaskedMemoryRead;
         }
 
+        /**
+         * Some EDBG AVR8 debug tools behave in a really weird way when servicing read memory requests that exceed a
+         * certain size. For example, the ATMEGA4809-XPRO Xplained Pro debug tool returns incorrect data for any read
+         * memory command that exceeds 256 bytes in the size of the read request, despite the fact that the HID report
+         * size is 512 bytes. The debug tool doesn't report any error, it just returns incorrect data.
+         *
+         * To address this, debug tool drivers can set a hard limit on the number of bytes this EdbgAvr8Interface instance
+         * will attempt to access in a single request, using the EdbgAvr8Interface::setMaximumMemoryAccessSizePerRequest()
+         * function.
+         *
+         * This limit will be enforced in all forms of memory access on the AVR8 target, including register access.
+         *
+         * @TODO: Enforce the limit on memory writes.
+         *
+         * @param maximumSize
+         */
+        void setMaximumMemoryAccessSizePerRequest(std::uint32_t maximumSize) {
+            this->maximumMemoryAccessSizePerRequest = maximumSize;
+        }
+
         /*
          * The public methods below implement the interface defined by the Avr8Interface class.
          * See the comments in that class for more info on the expected behaviour of each method.
@@ -275,6 +295,8 @@ namespace Bloom::DebugToolDrivers::Protocols::CmsisDap::Edbg::Avr
          */
         bool avoidMaskedMemoryRead = false;
 
+        std::optional<std::uint32_t> maximumMemoryAccessSizePerRequest;
+
         /**
          * We keep record of the current target state for caching purposes. We'll only refresh the target state if the
          * target is running. If it has already stopped, then we assume it cannot transition to a running state without