Applied debug-interface-specific access restrictions for memory and registers

src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.cpp

@@ -270,8 +270,8 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr
         }
     }
 
-    Targets::TargetRegisterAccess EdbgAvr8Interface::getRegisterAccess(
+    void EdbgAvr8Interface::applyAccessRestrictions(
-        const TargetRegisterDescriptor& registerDescriptor,
+        TargetRegisterDescriptor& registerDescriptor,
         const TargetAddressSpaceDescriptor& addressSpaceDescriptor
     ) {
         /*
@@ -281,10 +281,10 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr
          * There are some other memory types we can use to access some other registers during a debug session, but
          * these are yet to be implemented.
          */
-        const auto access = addressSpaceDescriptor.key == this->session.dataAddressSpace.key
+        const auto accessible = addressSpaceDescriptor.key == this->session.dataAddressSpace.key
             || addressSpaceDescriptor.key == this->session.registerFileAddressSpace.key;
-
+        registerDescriptor.access.readable = accessible && registerDescriptor.access.readable;
-        return {access, access};
+        registerDescriptor.access.writable = accessible && registerDescriptor.access.writable;
     }
 
     TargetMemoryAddress EdbgAvr8Interface::getProgramCounter() {

src/DebugToolDrivers/Microchip/Protocols/EDBG/AVR/EdbgAvr8Interface.hpp

@@ -138,8 +138,8 @@ namespace DebugToolDrivers::Microchip::Protocols::Edbg::Avr
          */
         void deactivate() override;
 
-        Targets::TargetRegisterAccess getRegisterAccess(
+        void applyAccessRestrictions(
-            const Targets::TargetRegisterDescriptor& registerDescriptor,
+            Targets::TargetRegisterDescriptor& registerDescriptor,
             const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor
         ) override;
 

src/DebugToolDrivers/TargetInterfaces/Microchip/AVR8/Avr8DebugInterface.hpp

@@ -70,8 +70,7 @@ namespace DebugToolDrivers::TargetInterfaces::Microchip::Avr8
          * registers can only be accessed during a programming session. This restriction is specific to the EDBG debug
          * interface.
          *
-         * This function should communicate any access restrictions for the given register, which apply during a debug
+         * This function should apply any additional access restrictions for the given register.
-         * session. It does not need to account for any access restrictions that only apply outside of a debug session.
          *
          * @param registerDescriptor
          *  The descriptor of the register.
@@ -81,8 +80,8 @@ namespace DebugToolDrivers::TargetInterfaces::Microchip::Avr8
          *
          * @return
          */
-        virtual Targets::TargetRegisterAccess getRegisterAccess(
+        virtual void applyAccessRestrictions(
-            const Targets::TargetRegisterDescriptor& registerDescriptor,
+            Targets::TargetRegisterDescriptor& registerDescriptor,
             const Targets::TargetAddressSpaceDescriptor& addressSpaceDescriptor
         ) = 0;
 

src/DebugToolDrivers/TargetInterfaces/RiscV/RiscVDebugInterface.hpp

@@ -59,5 +59,15 @@ namespace DebugToolDrivers::TargetInterfaces::RiscV
 
         virtual void enableProgrammingMode() = 0;
         virtual void disableProgrammingMode() = 0;
+
+        /**
+         * The debug interface may have its own access restrictions for memory segments and registers. These member
+         * functions should adjust the access members of the given segment/register descriptor, to apply any additional
+         * access restrictions.
+         *
+         * The member functions are called as part of the construction of the RISC-V target descriptor.
+         */
+        virtual void applyAccessRestrictions(Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor) = 0;
+        virtual void applyAccessRestrictions(Targets::TargetRegisterDescriptor& registerDescriptor) = 0;
     };
 }

src/DebugToolDrivers/WCH/WchLinkDebugInterface.cpp

@@ -357,6 +357,20 @@ namespace DebugToolDrivers::Wch
         this->softwareBreakpointRegistry.clear();
     }
 
+    void WchLinkDebugInterface::applyAccessRestrictions(TargetMemorySegmentDescriptor& memorySegmentDescriptor) {
+        if (memorySegmentDescriptor.type == TargetMemorySegmentType::FLASH) {
+            /*
+             * Actually, we *can* write to flash memory whilst in debug mode (via a partial page write), but we don't
+             * need to, so I'm just going to block it, for now.
+             */
+            memorySegmentDescriptor.debugModeAccess.writeable = false;
+        }
+    }
+
+    void WchLinkDebugInterface::applyAccessRestrictions(Targets::TargetRegisterDescriptor& registerDescriptor) {
+        // I don't believe any further access restrictions are required for registers. TODO: Review after v2.0.0.
+    }
+
     void WchLinkDebugInterface::setSoftwareBreakpoint(const TargetProgramBreakpoint& breakpoint) {
         if (breakpoint.size != 2 && breakpoint.size != 4) {
             throw Exception{"Invalid software breakpoint size (" + std::to_string(breakpoint.size) + ")"};

src/DebugToolDrivers/WCH/WchLinkDebugInterface.hpp

@@ -78,6 +78,9 @@ namespace DebugToolDrivers::Wch
         void enableProgrammingMode() override;
         void disableProgrammingMode() override;
 
+        void applyAccessRestrictions(Targets::TargetMemorySegmentDescriptor& memorySegmentDescriptor) override;
+        void applyAccessRestrictions(Targets::TargetRegisterDescriptor& registerDescriptor) override;
+
     private:
         const WchLinkToolConfig& toolConfig;
         const Targets::RiscV::RiscVTargetConfig& targetConfig;

src/Targets/Microchip/AVR8/Avr8.cpp

@@ -328,7 +328,7 @@ namespace Targets::Microchip::Avr8
         // The debug interface may have its own access restrictions for registers.
         for (auto& [peripheralKey, peripheral] : descriptor.peripheralDescriptorsByKey) {
             for (auto& [groupKey, registerGroup] : peripheral.registerGroupDescriptorsByKey) {
-                this->applyDebugInterfaceRegisterAccessRestrictions(
+                this->applyDebugInterfaceAccessRestrictions(
                     registerGroup,
                     descriptor.getAddressSpaceDescriptor(registerGroup.addressSpaceKey)
                 );
@@ -781,23 +781,16 @@ namespace Targets::Microchip::Avr8
         this->writeRegisters({{descriptor, value}});
     }
 
-    void Avr8::applyDebugInterfaceRegisterAccessRestrictions(
+    void Avr8::applyDebugInterfaceAccessRestrictions(
         TargetRegisterGroupDescriptor& groupDescriptor,
         const TargetAddressSpaceDescriptor& addressSpaceDescriptor
     ) {
-        for (auto& [subgroupKey, subgroupDescriptor] : groupDescriptor.subgroupDescriptorsByKey) {
-            this->applyDebugInterfaceRegisterAccessRestrictions(subgroupDescriptor, addressSpaceDescriptor);
-        }
-
         for (auto& [registerKey, registerDescriptor] : groupDescriptor.registerDescriptorsByKey) {
-            if (!registerDescriptor.access.readable && !registerDescriptor.access.writable) {
+            this->avr8DebugInterface->applyAccessRestrictions(registerDescriptor, addressSpaceDescriptor);
-                // This register is already inaccessible - no need to check for further restrictions
-                continue;
         }
 
-            const auto access = this->avr8DebugInterface->getRegisterAccess(registerDescriptor, addressSpaceDescriptor);
+        for (auto& [subgroupKey, subgroupDescriptor] : groupDescriptor.subgroupDescriptorsByKey) {
-            registerDescriptor.access.readable = registerDescriptor.access.readable && access.readable;
+            this->applyDebugInterfaceAccessRestrictions(subgroupDescriptor, addressSpaceDescriptor);
-            registerDescriptor.access.writable = registerDescriptor.access.writable && access.writable;
         }
     }
 

src/Targets/Microchip/AVR8/Avr8.hpp

@@ -169,7 +169,7 @@ namespace Targets::Microchip::Avr8
         TargetMemoryBuffer readRegister(const TargetRegisterDescriptor& descriptor);
         void writeRegister(const TargetRegisterDescriptor& descriptor, const TargetMemoryBuffer& value) ;
 
-        void applyDebugInterfaceRegisterAccessRestrictions(
+        void applyDebugInterfaceAccessRestrictions(
             TargetRegisterGroupDescriptor& groupDescriptor,
             const TargetAddressSpaceDescriptor& addressSpaceDescriptor
         );

src/Targets/RiscV/RiscV.cpp

@@ -307,6 +307,22 @@ namespace Targets::RiscV
         return this->programmingMode;
     }
 
+    void RiscV::applyDebugInterfaceAccessRestrictions(TargetAddressSpaceDescriptor& addressSpaceDescriptor) {
+        for (auto& [segmentKey, segmentDescriptor] : addressSpaceDescriptor.segmentDescriptorsByKey) {
+            this->riscVDebugInterface->applyAccessRestrictions(segmentDescriptor);
+        }
+    }
+
+    void RiscV::applyDebugInterfaceAccessRestrictions(TargetRegisterGroupDescriptor& registerGroupDescriptor) {
+        for (auto& [registerKey, registerDescriptor] : registerGroupDescriptor.registerDescriptorsByKey) {
+            this->riscVDebugInterface->applyAccessRestrictions(registerDescriptor);
+        }
+
+        for (auto& [subgroupKey, subgroupDescriptor] : registerGroupDescriptor.subgroupDescriptorsByKey) {
+            this->applyDebugInterfaceAccessRestrictions(subgroupDescriptor);
+        }
+    }
+
     const TargetMemorySegmentDescriptor& RiscV::resolveRegisterMemorySegmentDescriptor(
         const TargetRegisterDescriptor& regDescriptor,
         const TargetAddressSpaceDescriptor& addressSpaceDescriptor
@@ -355,6 +371,7 @@ namespace Targets::RiscV
                 false,
                 {true, true},
                 {false, false},
+                false,
                 std::nullopt
             }
         );
@@ -371,6 +388,7 @@ namespace Targets::RiscV
                 false,
                 {true, true},
                 {false, false},
+                false,
                 std::nullopt
             }
         );

src/Targets/RiscV/RiscV.hpp

@@ -11,6 +11,9 @@
 #include "TargetDescriptionFile.hpp"
 #include "IsaDescriptor.hpp"
 
+#include "src/Targets/TargetAddressSpaceDescriptor.hpp"
+#include "src/Targets/TargetRegisterGroupDescriptor.hpp"
+
 #include "src/DebugToolDrivers/TargetInterfaces/RiscV/RiscVDebugInterface.hpp"
 
 namespace Targets::RiscV
@@ -119,6 +122,9 @@ namespace Targets::RiscV
 
         bool programmingMode = false;
 
+        void applyDebugInterfaceAccessRestrictions(TargetAddressSpaceDescriptor& addressSpaceDescriptor);
+        void applyDebugInterfaceAccessRestrictions(TargetRegisterGroupDescriptor& registerGroupDescriptor);
+
         const TargetMemorySegmentDescriptor& resolveRegisterMemorySegmentDescriptor(
             const TargetRegisterDescriptor& regDescriptor,
             const TargetAddressSpaceDescriptor& addressSpaceDescriptor

src/Targets/RiscV/Wch/WchRiscV.cpp

@@ -78,6 +78,16 @@ namespace Targets::RiscV::Wch
             this->cpuPeripheralDescriptor.clone()
         );
 
+        for (auto& [addressSpaceKey, addressSpaceDescriptor] : descriptor.addressSpaceDescriptorsByKey) {
+            this->applyDebugInterfaceAccessRestrictions(addressSpaceDescriptor);
+        }
+
+        for (auto& [peripheralKey, peripheralDescriptor] : descriptor.peripheralDescriptorsByKey) {
+            for (auto& [groupKey, groupDescriptor] : peripheralDescriptor.registerGroupDescriptorsByKey) {
+                this->applyDebugInterfaceAccessRestrictions(groupDescriptor);
+            }
+        }
+
         auto& sysAddressSpaceDescriptor = descriptor.getAddressSpaceDescriptor("system");
         sysAddressSpaceDescriptor.getMemorySegmentDescriptor("internal_program_memory").inspectionEnabled = true;
         sysAddressSpaceDescriptor.getMemorySegmentDescriptor("internal_ram").inspectionEnabled = true;