From dd0a1430da3c55a2354810e8fedf8b9917f8b323 Mon Sep 17 00:00:00 2001 From: Zoe Roux Date: Thu, 8 Jul 2021 20:22:25 +0200 Subject: [PATCH] Fixing DMA write/read --- sources/CPU/CPU.cpp | 4 ++-- sources/Debugger/CPU/CPUDebug.cpp | 1 + sources/Debugger/MemoryBusDebug.cpp | 4 +++- sources/Debugger/MemoryViewer.cpp | 23 +++++++++----------- sources/Exceptions/InvalidAddress.hpp | 30 ++++++++++++++++++++------- sources/Memory/IMemory.hpp | 12 +++++------ tests/testMemoryBus.cpp | 11 ++++++++++ 7 files changed, 55 insertions(+), 30 deletions(-) diff --git a/sources/CPU/CPU.cpp b/sources/CPU/CPU.cpp index e932e21..df9e0a8 100644 --- a/sources/CPU/CPU.cpp +++ b/sources/CPU/CPU.cpp @@ -94,7 +94,7 @@ namespace ComSquare::CPU case 0x1F: return this->_internalRegisters.joy4h; case 0x100 ... 0x180: - return this->_dmaChannels[(addr - 0x100) >> 8u].read(addr & 0xF); + return this->_dmaChannels[(addr - 0x100) >> 4u].read(addr & 0xF); default: throw InvalidAddress("CPU Internal Registers read", addr + this->_start); } @@ -195,7 +195,7 @@ namespace ComSquare::CPU this->_internalRegisters.joy4h = data; break; case 0x100 ... 0x180: - this->_dmaChannels[(addr - 0x100) >> 8u].write(addr & 0xF, data); + this->_dmaChannels[(addr - 0x100) >> 4u].write(addr & 0xF, data); break; default: throw InvalidAddress("CPU Internal Registers write", addr + this->_start); diff --git a/sources/Debugger/CPU/CPUDebug.cpp b/sources/Debugger/CPU/CPUDebug.cpp index 3fe1b6d..6f0ed1a 100644 --- a/sources/Debugger/CPU/CPUDebug.cpp +++ b/sources/Debugger/CPU/CPUDebug.cpp @@ -120,6 +120,7 @@ namespace ComSquare::Debugger::CPU std::cerr << "An error occurred: " << e.what() << std::endl; QApplication::quit(); } + return 0xFF; } void CPUDebug::_logInstruction() diff --git a/sources/Debugger/MemoryBusDebug.cpp b/sources/Debugger/MemoryBusDebug.cpp index c6c6040..65ce5d3 100644 --- a/sources/Debugger/MemoryBusDebug.cpp +++ b/sources/Debugger/MemoryBusDebug.cpp @@ -197,8 +197,10 @@ namespace ComSquare::Debugger case 2: return QString(log.accessor ? log.accessor->getName().c_str() : "Bus"); case 3: { + if (!log.accessor) + return QString("Open bus"); uint24_t addr = log.accessor->getRelativeAddress(log.addr); - return QString(log.accessor ? log.accessor->getValueName(addr).c_str() : "Open bus"); + return QString(log.accessor->getValueName(addr).c_str()); } case 4: if (!log.oldData) diff --git a/sources/Debugger/MemoryViewer.cpp b/sources/Debugger/MemoryViewer.cpp index d60d8ba..478ce09 100644 --- a/sources/Debugger/MemoryViewer.cpp +++ b/sources/Debugger/MemoryViewer.cpp @@ -136,10 +136,11 @@ namespace ComSquare::Debugger if (dialogUI.checkBox->isChecked()) { try { value = this->switchToAddrTab(value); - } catch (const InvalidAddress &) { + } catch (const InvalidAddress &ex) { QMessageBox msgBox; msgBox.setIcon(QMessageBox::Critical); - msgBox.setText("This address is not mapped. Reading it will result in OpenBus."); + msgBox.setWindowTitle(("Invalid address: " + Utility::to_hex(ex.getAddress())).c_str()); + msgBox.setText(ex.getWhere().c_str()); msgBox.exec(); return; } @@ -154,7 +155,8 @@ namespace ComSquare::Debugger { Memory::IMemory *accessor = this->_bus.getAccessor(addr); if (!accessor) - throw InvalidAddress("Memory viewer switch to address", addr); + throw InvalidAddress("This address is not mapped. Reading it will result in OpenBus.", addr); + uint24_t localAddr = accessor->getRelativeAddress(addr); switch (accessor->getComponent()) { case WRam: this->_ui.tabs->setCurrentIndex(0); @@ -166,17 +168,12 @@ namespace ComSquare::Debugger this->_ui.tabs->setCurrentIndex(2); break; default: - throw InvalidAddress("Memory viewer switch to address", addr); + throw InvalidAddress("The viewer can't display the value " + accessor->getValueName(localAddr) + + " of " + accessor->getName(), addr); } - addr = accessor->getRelativeAddress(addr); - if (addr > accessor->getSize()) { - QMessageBox msgBox; - msgBox.setIcon(QMessageBox::Critical); - msgBox.setText((std::string("The ") + accessor->getName() + " is too small to contain this address.").c_str()); - msgBox.exec(); - return 0; - } - return addr; + if (localAddr > accessor->getSize()) + throw InvalidAddress("The " + accessor->getName() + " is too small to contain this address.", addr); + return localAddr; } void MemoryViewer::focus() diff --git a/sources/Exceptions/InvalidAddress.hpp b/sources/Exceptions/InvalidAddress.hpp index d1bec40..f9788d8 100644 --- a/sources/Exceptions/InvalidAddress.hpp +++ b/sources/Exceptions/InvalidAddress.hpp @@ -10,6 +10,7 @@ #include #include #include +#include "Utility/Utility.hpp" namespace ComSquare { @@ -18,15 +19,28 @@ namespace ComSquare { private: std::string _msg; - + uint24_t _addr; + std::string _where; public: - InvalidAddress(std::string where, uint24_t addr) + + InvalidAddress(const std::string &where, uint24_t addr) + : _addr(addr), + _where(where) { - std::stringstream stream; - stream << "Could not read/write data at address: " << addr << " from " << where; - this->_msg = stream.str(); + this->_msg = "Could not read/write data at address: " + Utility::to_hex(addr) + " from " + where; + } + + [[nodiscard]] const char *what() const noexcept override { return this->_msg.c_str(); } + + [[nodiscard]] uint24_t getAddress() const + { + return this->_addr; + } + + [[nodiscard]] std::string getWhere() const + { + return this->_where; } - const char *what() const noexcept override { return this->_msg.c_str(); } }; //! @brief Exception thrown when trying to read/write to an invalid address in a rectangle memory region. @@ -36,7 +50,7 @@ namespace ComSquare std::string _msg; public: - InvalidRectangleAddress(std::string where, int32_t addr, int16_t subaddr, int16_t start, int16_t end) + InvalidRectangleAddress(const std::string &where, int32_t addr, int16_t subaddr, int16_t start, int16_t end) { std::stringstream stream; stream << "Could not read/write data at address: 0x" << std::hex << addr << " from " << where; @@ -46,6 +60,6 @@ namespace ComSquare stream << " (" << std::hex << subaddr << " > " << end << ")"; this->_msg = stream.str(); } - const char *what() const noexcept override { return this->_msg.c_str(); } + [[nodiscard]] const char *what() const noexcept override { return this->_msg.c_str(); } }; }// namespace ComSquare diff --git a/sources/Memory/IMemory.hpp b/sources/Memory/IMemory.hpp index ef4647b..fea5b3b 100644 --- a/sources/Memory/IMemory.hpp +++ b/sources/Memory/IMemory.hpp @@ -32,26 +32,26 @@ namespace ComSquare::Memory //! @brief Return true if this component has mapped the address. //! @param addr The address to check. //! @return True if this address is mapped to the component. False otherwise. - virtual bool hasMemoryAt(uint24_t addr) const = 0; + [[nodiscard]] virtual bool hasMemoryAt(uint24_t addr) const = 0; //! @brief Translate an absolute address to a relative address //! @param addr The absolute address (in the 24 bit bus) //! @return The local address (0 refers to the first byte of this component). //! @throw InvalidAddress is thrown if the address is not mapped by this component. - virtual uint24_t getRelativeAddress(uint24_t addr) const = 0; + [[nodiscard]] virtual uint24_t getRelativeAddress(uint24_t addr) const = 0; //! @brief Get the size of the data. This size can be lower than the mapped data. //! @return The number of bytes inside this memory. - virtual uint24_t getSize() const = 0; + [[nodiscard]] virtual uint24_t getSize() const = 0; //! @brief Get the name of this accessor (used for debug purpose) - virtual std::string getName() const = 0; + [[nodiscard]] virtual std::string getName() const = 0; //! @brief Get the component of this accessor (used for debug purpose) - virtual Component getComponent() const = 0; + [[nodiscard]] virtual Component getComponent() const = 0; //! @brief Get the name of the data at the address //! @param addr The address (in local space) - virtual std::string getValueName(uint24_t addr) const = 0; + [[nodiscard]] virtual std::string getValueName(uint24_t addr) const = 0; //! @brief A virtual default destructor. virtual ~IMemory() = default; diff --git a/tests/testMemoryBus.cpp b/tests/testMemoryBus.cpp index 1f35a35..aaf4305 100644 --- a/tests/testMemoryBus.cpp +++ b/tests/testMemoryBus.cpp @@ -418,4 +418,15 @@ TEST_CASE("WriteSRAM BusWrite", "[BusWrite]") snes.bus.write(0x700009, 123); REQUIRE(snes.sram._data[9] == 123); +} + +TEST_CASE("WriteDMA BusWrite", "[BusWrite]") +{ + Init() + + snes.bus.write(0x4372, 123); + REQUIRE(snes.bus.read(0x4372) == 123); + REQUIRE(snes.cpu._dmaChannels[7]._aAddress.bytes[0] == 123); + snes.bus.write(0x4373, 31); + REQUIRE(snes.cpu._dmaChannels[7]._aAddress.bytes[1] == 31); } \ No newline at end of file