Moved RISC-V CSR and GPR address spaces to TDF.

Some other bits of refactoring/tidying

src/DebugServer/Gdb/RiscVGdb/RiscVGdbTargetDescriptor.cpp

@@ -10,10 +10,10 @@ namespace DebugServer::Gdb::RiscVGdb
     using Exceptions::Exception;
 
     RiscVGdbTargetDescriptor::RiscVGdbTargetDescriptor(const Targets::TargetDescriptor& targetDescriptor)
-        : systemAddressSpaceDescriptor(targetDescriptor.getAddressSpaceDescriptor("system"))
+        : gprAddressSpaceDescriptor(targetDescriptor.getAddressSpaceDescriptor("gpr"))
-        , cpuAddressSpaceDescriptor(targetDescriptor.getAddressSpaceDescriptor("csr"))
+        , systemAddressSpaceDescriptor(targetDescriptor.getAddressSpaceDescriptor("system"))
         , programMemorySegmentDescriptor(this->systemAddressSpaceDescriptor.getMemorySegmentDescriptor("main_program"))
-        , gpRegistersMemorySegmentDescriptor(this->cpuAddressSpaceDescriptor.getMemorySegmentDescriptor("gp_registers"))
+        , gpRegistersMemorySegmentDescriptor(this->gprAddressSpaceDescriptor.getMemorySegmentDescriptor("gpr"))
         , cpuGpPeripheralDescriptor(targetDescriptor.getPeripheralDescriptor("cpu"))
         , cpuGpRegisterGroupDescriptor(this->cpuGpPeripheralDescriptor.getRegisterGroupDescriptor("gpr"))
         , programCounterGdbRegisterId(static_cast<GdbRegisterId>(this->cpuGpRegisterGroupDescriptor.registerDescriptorsByKey.size()))

src/DebugServer/Gdb/RiscVGdb/RiscVGdbTargetDescriptor.hpp

@@ -13,8 +13,8 @@ namespace DebugServer::Gdb::RiscVGdb
     class RiscVGdbTargetDescriptor: public DebugServer::Gdb::TargetDescriptor
     {
     public:
+        const Targets::TargetAddressSpaceDescriptor& gprAddressSpaceDescriptor;
         const Targets::TargetAddressSpaceDescriptor& systemAddressSpaceDescriptor;
-        const Targets::TargetAddressSpaceDescriptor& cpuAddressSpaceDescriptor;
 
         const Targets::TargetMemorySegmentDescriptor& programMemorySegmentDescriptor;
         const Targets::TargetMemorySegmentDescriptor& gpRegistersMemorySegmentDescriptor;

src/DebugServer/ServerInterface.hpp

@@ -4,9 +4,6 @@
 
 namespace DebugServer
 {
-    /**
-     * Every debug server must implement this interface.
-     */
     class ServerInterface
     {
     public:

src/Targets/RiscV/RiscV.cpp

@@ -21,13 +21,15 @@ namespace Targets::RiscV
         : targetConfig(RiscVTargetConfig{targetConfig})
         , targetDescriptionFile(targetDescriptionFile)
         , isaDescriptor(this->targetDescriptionFile.getIsaDescriptor())
-        , cpuRegisterAddressSpaceDescriptor(RiscV::generateCpuRegisterAddressSpaceDescriptor())
+        , csrAddressSpaceDescriptor(this->targetDescriptionFile.getCsrAddressSpaceDescriptor())
-        , csrMemorySegmentDescriptor(this->cpuRegisterAddressSpaceDescriptor.getMemorySegmentDescriptor("cs_registers"))
+        , csrMemorySegmentDescriptor(this->csrAddressSpaceDescriptor.getMemorySegmentDescriptor("csr"))
-        , gprMemorySegmentDescriptor(this->cpuRegisterAddressSpaceDescriptor.getMemorySegmentDescriptor("gp_registers"))
+        , gprAddressSpaceDescriptor(this->targetDescriptionFile.getGprAddressSpaceDescriptor())
+        , gprMemorySegmentDescriptor(this->gprAddressSpaceDescriptor.getMemorySegmentDescriptor("gpr"))
         , cpuPeripheralDescriptor(
             RiscV::generateCpuPeripheralDescriptor(
                 this->isaDescriptor,
-                this->cpuRegisterAddressSpaceDescriptor,
+                this->csrAddressSpaceDescriptor,
+                this->gprAddressSpaceDescriptor,
                 this->csrMemorySegmentDescriptor,
                 this->gprMemorySegmentDescriptor
             )
@@ -89,13 +91,25 @@ namespace Targets::RiscV
         auto cpuRegisterDescriptors = TargetRegisterDescriptors{};
 
         for (const auto& descriptor : descriptors) {
-            if (descriptor->addressSpaceId == this->cpuRegisterAddressSpaceDescriptor.id) {
+            if (
+                descriptor->addressSpaceId == this->csrAddressSpaceDescriptor.id
+                || descriptor->addressSpaceId == this->gprAddressSpaceDescriptor.id
+            ) {
                 if (
-                    !this->csrMemorySegmentDescriptor.addressRange.contains(descriptor->startAddress)
+                    descriptor->addressSpaceId == this->csrAddressSpaceDescriptor.id
+                    && !this->csrMemorySegmentDescriptor.addressRange.contains(descriptor->startAddress)
+                ) {
+                    throw Exceptions::Exception{
+                        "Cannot access CPU CSR `" + descriptor->key + "` - unknown memory segment"
+                    };
+                }
+
+                if (
+                    descriptor->addressSpaceId == this->gprAddressSpaceDescriptor.id
                     && !this->gprMemorySegmentDescriptor.addressRange.contains(descriptor->startAddress)
                 ) {
                     throw Exceptions::Exception{
-                        "Cannot access CPU register `" + descriptor->key + "` - unknown memory segment"
+                        "Cannot access CPU GPR `" + descriptor->key + "` - unknown memory segment"
                     };
                 }
 
@@ -136,7 +150,7 @@ namespace Targets::RiscV
         for (const auto& pair : registers) {
             const auto& descriptor = pair.first;
 
-            if (descriptor.addressSpaceId == this->cpuRegisterAddressSpaceDescriptor.id) {
+            if (descriptor.addressSpaceId == this->csrAddressSpaceDescriptor.id) {
                 if (
                     !this->csrMemorySegmentDescriptor.addressRange.contains(descriptor.startAddress)
                     && !this->gprMemorySegmentDescriptor.addressRange.contains(descriptor.startAddress)
@@ -304,6 +318,10 @@ namespace Targets::RiscV
         return DynamicRegisterValue{descriptor, this->readRegister(descriptor)};
     }
 
+    void RiscV::writeRegister(const DynamicRegisterValue& dynamicRegister) {
+        this->writeRegister(dynamicRegister.registerDescriptor, dynamicRegister.data());
+    }
+
     void RiscV::writeRegister(const TargetRegisterDescriptor& descriptor, TargetMemoryBufferSpan value) {
         this->writeRegisters({{descriptor, TargetMemoryBuffer{value.begin(), value.end()}}});
     }
@@ -374,55 +392,10 @@ namespace Targets::RiscV
         return *(segmentDescriptors.front());
     }
 
-    TargetAddressSpaceDescriptor RiscV::generateCpuRegisterAddressSpaceDescriptor() {
-        auto addressSpace = TargetAddressSpaceDescriptor{
-            "csr",
-            {0x0000, 0xFFFF},
-            TargetMemoryEndianness::LITTLE,
-            {},
-            4
-        };
-
-        addressSpace.segmentDescriptorsByKey.emplace(
-            "cs_registers",
-            TargetMemorySegmentDescriptor{
-                addressSpace.key,
-                "cs_registers",
-                "Control Status Registers",
-                TargetMemorySegmentType::REGISTERS,
-                {0x0000, 0x0FFF},
-                addressSpace.unitSize,
-                false,
-                {true, true},
-                {false, false},
-                false,
-                std::nullopt
-            }
-        );
-
-        addressSpace.segmentDescriptorsByKey.emplace(
-            "gp_registers",
-            TargetMemorySegmentDescriptor{
-                addressSpace.key,
-                "gp_registers",
-                "General Purpose Registers",
-                TargetMemorySegmentType::GENERAL_PURPOSE_REGISTERS,
-                {0x1000, 0x101F},
-                addressSpace.unitSize,
-                false,
-                {true, true},
-                {false, false},
-                false,
-                std::nullopt
-            }
-        );
-
-        return addressSpace;
-    }
-
     TargetPeripheralDescriptor RiscV::generateCpuPeripheralDescriptor(
         const IsaDescriptor& isaDescriptor,
-        const TargetAddressSpaceDescriptor& addressSpaceDescriptor,
+        const TargetAddressSpaceDescriptor& csrAddressSpaceDescriptor,
+        const TargetAddressSpaceDescriptor& gprAddressSpaceDescriptor,
         const TargetMemorySegmentDescriptor& csrMemorySegmentDescriptor,
         const TargetMemorySegmentDescriptor& gprMemorySegmentDescriptor
     ) {
@@ -441,7 +414,7 @@ namespace Targets::RiscV
                 "gpr",
                 "GPR",
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                gprAddressSpaceDescriptor.key,
                 std::nullopt,
                 {},
                 {}
@@ -457,7 +430,7 @@ namespace Targets::RiscV
                     "X" + std::to_string(i),
                     gprGroup.absoluteKey,
                     cpuPeripheralDescriptor.key,
-                    addressSpaceDescriptor.key,
+                    gprAddressSpaceDescriptor.key,
                     gprMemorySegmentDescriptor.addressRange.startAddress + i,
                     4,
                     TargetRegisterType::GENERAL_PURPOSE_REGISTER,
@@ -475,7 +448,7 @@ namespace Targets::RiscV
                 "csr",
                 "CSR",
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 std::nullopt,
                 {},
                 {}
@@ -489,7 +462,7 @@ namespace Targets::RiscV
                 "MARCHID",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0xF12,
                 4,
                 TargetRegisterType::OTHER,
@@ -506,7 +479,7 @@ namespace Targets::RiscV
                 "MIMPID",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0xF13,
                 4,
                 TargetRegisterType::OTHER,
@@ -523,7 +496,7 @@ namespace Targets::RiscV
                 "MSTATUS",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x300,
                 4,
                 TargetRegisterType::OTHER,
@@ -540,7 +513,7 @@ namespace Targets::RiscV
                 "MISA",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x301,
                 4,
                 TargetRegisterType::OTHER,
@@ -557,7 +530,7 @@ namespace Targets::RiscV
                 "MTVEC",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x305,
                 4,
                 TargetRegisterType::OTHER,
@@ -574,7 +547,7 @@ namespace Targets::RiscV
                 "MCOUNTEREN",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x306,
                 4,
                 TargetRegisterType::OTHER,
@@ -591,7 +564,7 @@ namespace Targets::RiscV
                 "MSCRATCH",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x340,
                 4,
                 TargetRegisterType::OTHER,
@@ -608,7 +581,7 @@ namespace Targets::RiscV
                 "MEPC",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x341,
                 4,
                 TargetRegisterType::OTHER,
@@ -625,7 +598,7 @@ namespace Targets::RiscV
                 "MCAUSE",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x342,
                 4,
                 TargetRegisterType::OTHER,
@@ -642,7 +615,7 @@ namespace Targets::RiscV
                 "MTVAL",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x343,
                 4,
                 TargetRegisterType::OTHER,
@@ -659,7 +632,7 @@ namespace Targets::RiscV
                 "MIP",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x344,
                 4,
                 TargetRegisterType::OTHER,
@@ -676,7 +649,7 @@ namespace Targets::RiscV
                 "DCSR",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x7B0,
                 4,
                 TargetRegisterType::OTHER,
@@ -693,7 +666,7 @@ namespace Targets::RiscV
                 "DPC",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x7B1,
                 4,
                 TargetRegisterType::OTHER,
@@ -710,7 +683,7 @@ namespace Targets::RiscV
                 "DSCRATCH0",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x7B2,
                 4,
                 TargetRegisterType::OTHER,
@@ -727,7 +700,7 @@ namespace Targets::RiscV
                 "DSCRATCH1",
                 csrGroup.absoluteKey,
                 cpuPeripheralDescriptor.key,
-                addressSpaceDescriptor.key,
+                csrAddressSpaceDescriptor.key,
                 csrMemorySegmentDescriptor.addressRange.startAddress + 0x7B3,
                 4,
                 TargetRegisterType::OTHER,

src/Targets/RiscV/RiscV.hpp

@@ -88,23 +88,9 @@ namespace Targets::RiscV
 
         DebugToolDrivers::TargetInterfaces::RiscV::RiscVDebugInterface* riscVDebugInterface = nullptr;
 
-        /*
+        TargetAddressSpaceDescriptor csrAddressSpaceDescriptor;
-         * On RISC-V targets, CPU registers are typically only accessible via the debug module (we can't access them
-         * via the system address space). So we use abstract commands to access these registers. This means we have to
-         * address these registers via their register numbers, as defined in the RISC-V debug spec.
-         *
-         * We effectively treat register numbers as a separate address space, with an addressable unit size of 4 bytes.
-         * The `cpuRegisterAddressSpaceDescriptor` member holds the descriptor for this address space.
-         *
-         * TODO: review this. This address space is specific to the RISC-V debug spec, but some debug tools may
-         *       implement their own debug translator in firmware, and then provide a higher-level API to access the
-         *       same registers. In that case, this address space may not be relevant. This may need to be moved.
-         *       ATM all RISC-V debug tools supported by Bloom provide a DTM interface, so we use our own debug
-         *       translator driver and this address space is, in fact, relevant. I will deal with this when it
-         *       becomes a problem.
-         */
-        TargetAddressSpaceDescriptor cpuRegisterAddressSpaceDescriptor;
         const TargetMemorySegmentDescriptor& csrMemorySegmentDescriptor;
+        TargetAddressSpaceDescriptor gprAddressSpaceDescriptor;
         const TargetMemorySegmentDescriptor& gprMemorySegmentDescriptor;
 
         TargetPeripheralDescriptor cpuPeripheralDescriptor;
@@ -122,6 +108,7 @@ namespace Targets::RiscV
 
         TargetMemoryBuffer readRegister(const TargetRegisterDescriptor& descriptor);
         DynamicRegisterValue readRegisterDynamicValue(const TargetRegisterDescriptor& descriptor);
+        void writeRegister(const DynamicRegisterValue& dynamicRegister);
         void writeRegister(const TargetRegisterDescriptor& descriptor, TargetMemoryBufferSpan value);
         void writeRegister(const TargetRegisterDescriptor& descriptor, std::uint64_t value);
 
@@ -139,10 +126,10 @@ namespace Targets::RiscV
             const TargetAddressSpaceDescriptor& addressSpaceDescriptor
         );
 
-        static TargetAddressSpaceDescriptor generateCpuRegisterAddressSpaceDescriptor();
         static TargetPeripheralDescriptor generateCpuPeripheralDescriptor(
             const IsaDescriptor& isaDescriptor,
-            const TargetAddressSpaceDescriptor& addressSpaceDescriptor,
+            const TargetAddressSpaceDescriptor& csrAddressSpaceDescriptor,
+            const TargetAddressSpaceDescriptor& gprAddressSpaceDescriptor,
             const TargetMemorySegmentDescriptor& csrMemorySegmentDescriptor,
             const TargetMemorySegmentDescriptor& gprMemorySegmentDescriptor
         );

src/Targets/RiscV/TargetDescriptionFile.cpp

@@ -6,10 +6,26 @@ namespace Targets::RiscV
         : Targets::TargetDescription::TargetDescriptionFile(xmlFilePath)
     {}
 
+    const TargetDescription::AddressSpace& TargetDescriptionFile::getCsrAddressSpace() const {
+        return this->getAddressSpace("csr");
+    }
+
+    const TargetDescription::AddressSpace& TargetDescriptionFile::getGprAddressSpace() const {
+        return this->getAddressSpace("gpr");
+    }
+
     const TargetDescription::AddressSpace& TargetDescriptionFile::getSystemAddressSpace() const {
         return this->getAddressSpace("system");
     }
 
+    TargetAddressSpaceDescriptor TargetDescriptionFile::getCsrAddressSpaceDescriptor() const {
+        return this->targetAddressSpaceDescriptorFromAddressSpace(this->getCsrAddressSpace());
+    }
+
+    TargetAddressSpaceDescriptor TargetDescriptionFile::getGprAddressSpaceDescriptor() const {
+        return this->targetAddressSpaceDescriptorFromAddressSpace(this->getGprAddressSpace());
+    }
+
     TargetAddressSpaceDescriptor TargetDescriptionFile::getSystemAddressSpaceDescriptor() const {
         return this->targetAddressSpaceDescriptorFromAddressSpace(this->getSystemAddressSpace());
     }

src/Targets/RiscV/TargetDescriptionFile.hpp

@@ -11,7 +11,12 @@ namespace Targets::RiscV
     public:
         explicit TargetDescriptionFile(const std::string& xmlFilePath);
 
+        [[nodiscard]] const TargetDescription::AddressSpace& getCsrAddressSpace() const;
+        [[nodiscard]] const TargetDescription::AddressSpace& getGprAddressSpace() const;
         [[nodiscard]] const TargetDescription::AddressSpace& getSystemAddressSpace() const;
+
+        [[nodiscard]] TargetAddressSpaceDescriptor getCsrAddressSpaceDescriptor() const;
+        [[nodiscard]] TargetAddressSpaceDescriptor getGprAddressSpaceDescriptor() const;
         [[nodiscard]] TargetAddressSpaceDescriptor getSystemAddressSpaceDescriptor() const;
         [[nodiscard]] IsaDescriptor getIsaDescriptor() const;
     };

src/Targets/RiscV/Wch/WchRiscV.cpp

@@ -129,12 +129,6 @@ namespace Targets::RiscV::Wch
             descriptor.breakpointResources.reservedHardwareBreakpoints = 1;
         }
 
-        // Copy the RISC-V CPU register address space and peripheral descriptor
-        descriptor.addressSpaceDescriptorsByKey.emplace(
-            this->cpuRegisterAddressSpaceDescriptor.key,
-            this->cpuRegisterAddressSpaceDescriptor.clone()
-        );
-
         descriptor.peripheralDescriptorsByKey.emplace(
             this->cpuPeripheralDescriptor.key,
             this->cpuPeripheralDescriptor.clone()
@@ -457,7 +451,7 @@ namespace Targets::RiscV::Wch
                 )
             );
 
-            this->writeRegister(gpioPadDescriptor.configRegisterDescriptor, configRegisterValue.data());
+            this->writeRegister(configRegisterValue);
         }
 
         if (state.direction == TargetGpioPadState::DataDirection::OUTPUT) {
@@ -468,7 +462,7 @@ namespace Targets::RiscV::Wch
                     ? 0x01
                     : 0x00
             );
-            this->writeRegister(gpioPadDescriptor.outputDataRegisterDescriptor, outDataRegisterValue.data());
+            this->writeRegister(outDataRegisterValue);
         }
     }
 
@@ -663,7 +657,7 @@ namespace Targets::RiscV::Wch
         }
 
         statusRegister.setBitField(this->flashStatusBootModeFieldDescriptor, true);
-        this->writeRegister(this->flashStatusRegisterDescriptor, statusRegister.data());
+        this->writeRegister(statusRegister);
 
         this->reset();
     }
@@ -679,7 +673,7 @@ namespace Targets::RiscV::Wch
         }
 
         statusRegister.setBitField(this->flashStatusBootModeFieldDescriptor, false);
-        this->writeRegister(this->flashStatusRegisterDescriptor, statusRegister.data());
+        this->writeRegister(statusRegister);
 
         this->reset();
     }

src/Targets/TargetDescription/TargetDescriptionFile.cpp

@@ -625,6 +625,7 @@ namespace Targets::TargetDescription
     MemorySegment TargetDescriptionFile::memorySegmentFromXml(const QDomElement& xmlElement) {
         static const auto typesByName = BiMap<std::string, TargetMemorySegmentType>{
             {"gp_registers", TargetMemorySegmentType::GENERAL_PURPOSE_REGISTERS},
+            {"registers", TargetMemorySegmentType::REGISTERS},
             {"aliased", TargetMemorySegmentType::ALIASED},
             {"eeprom", TargetMemorySegmentType::EEPROM},
             {"flash", TargetMemorySegmentType::FLASH},