riscv: Heavy bus refactor.

It now doesn't assume literally every device would map something to memory.

This should also fix some API orthogonality issues (ergo the CPU being treated specially)

native/CMakeLists.txt

@@ -9,6 +9,8 @@ add_subdirectory(projects/lucore)
 # RISC-V emulator library
 add_subdirectory(projects/riscv)
 
+add_subdirectory(projects/riscv_test_harness)
+
 # Garry's Mod native bindings to RISC-V emulator
 # Also lua device stuff
 add_subdirectory(projects/lcpu)

native/projects/lucore/include/lucore/OptionalRef.hpp

@@ -1,4 +1,5 @@
 //! OptionalRef - std::optional<T&> for C++20
+#pragma once
 
 #include <lucore/Assert.hpp>
 

native/projects/riscv/CMakeLists.txt

@@ -7,6 +7,7 @@ project(riscv_emu
 
 add_library(riscv
 	src/Bus.cpp
+	src/CPU.cpp
 	src/Devices/RamDevice.cpp
 )
 

native/projects/riscv/include/riscv/Bus.hpp

@@ -1,4 +1,6 @@
-#include <lucore/OptionalRef.hpp>
+#pragma once
+
+#include <lucore/Assert.hpp>
 #include <riscv/Types.hpp>
 #include <unordered_map>
 #include <vector>
@@ -7,51 +9,94 @@ namespace riscv {
 
 	struct CPU;
 
-	/// An address/memory bus. No virtual address translation is implemented;
+	/// An bus that is part of a [System].
-	/// all addresses/devices are placed in physical addresses.
+	/// Note that no virtual address translation is implemented at all.
+	/// All addresses/devices of devices in memory are placed in physical addresses.
 	struct Bus {
-		/// Interface all memory bus devices use.
 		struct Device {
-			Device() = default;
+			enum class BasicType {
+				Device,		 // do not upcast.
+				PlainMemory, // upcast to MemoryDevice is safe.
+				Mmio		 // upcast to MmioDevice is safe.
+			};
 
-			// Devices have no need to be copied or moved.
+			Device() = default;
 			Device(const Device&) = delete;
 			Device(Device&&) = delete;
 
 			virtual ~Device() = default;
 
-			/// How many bytes does this device occupy of address space? This should
+			virtual BasicType Type() const { return BasicType::Device; }
-			/// effectively be a constant, and should probably not change during CPU execution.
-			virtual AddressT Size() const = 0;
 
-			/// Used to allow bus devices to know when they are attached to a memory bus,
+			/// Does this device clock?
-			/// and ultimately, an instance of a System
-			virtual void Attached(Bus* memoryBus, AddressT baseAddress) = 0;
-
-			/// Does this device require a clock "signal"?
 			virtual bool Clocked() const { return false; }
 
-			/// This function is called to give clocked devices
+			/// This function is called by the bus to clock devices.
-			/// the ability to... well, clock!
 			virtual void Clock() {}
 
-			// TODO(feat): default implementations of Peek* and Poke* should exist
+			template <class T>
-			// and trap the CPU (similarly to what happens if a unmapped bus read occurs).
+			constexpr bool IsA() {
+				if(std::is_same_v<T, Bus::MemoryDevice*>) {
+					return this->Type() == BasicType::PlainMemory;
+				} else if(std::is_same_v<T, Bus::MmioDevice*>) {
+					return this->Type() == BasicType::Mmio;
+				}
+
+				// Invalid types should do this.
+				return false;
+			}
+
+			template <class T>
+			constexpr T Upcast() {
+				LUCORE_ASSERT(IsA<T>(), "Upcast failure: this is not a T");
+				return static_cast<T>(this);
+			}
+		};
+
+		/// Interface plain memory bus devices use.
+		struct MemoryDevice : Device {
+			virtual ~MemoryDevice() = default;
+
+			virtual BasicType Type() const override { return BasicType::PlainMemory; }
+
+			virtual AddressT Base() const = 0;
+			/// How many bytes does this device occupy of address space? This should
+			/// effectively be a constant, and should not change during CPU execution.
+			virtual AddressT Size() const = 0;
 
 			/// Peek() -> reads a value from this device.
-			virtual u8 PeekByte(AddressT offset) = 0;
+			virtual u8 PeekByte(AddressT address) = 0;
-			virtual u16 PeekShort(AddressT offset) = 0;
+			virtual u16 PeekShort(AddressT address) = 0;
-			virtual u32 PeekWord(AddressT offset) = 0;
+			virtual u32 PeekWord(AddressT address) = 0;
 
 			/// Poke() -> Writes a value to this device's space in memory
-			virtual void PokeByte(AddressT offset, u8 value) = 0;
+			virtual void PokeByte(AddressT address, u8 value) = 0;
-			virtual void PokeShort(AddressT offset, u16 value) = 0;
+			virtual void PokeShort(AddressT address, u16 value) = 0;
-			virtual void PokeWord(AddressT offset, u32 value) = 0;
+			virtual void PokeWord(AddressT address, u32 value) = 0;
+		};
+
+		/// A device in the MMIO region.
+		/// (0x10000000-0x12000000)
+		struct MmioDevice : Device {
+			virtual ~MmioDevice() = default;
+
+			virtual BasicType Type() const override { return BasicType::Mmio; }
+
+			virtual AddressT Base() const;
+
+			/// How many bytes does this device occupy of address space? This should
+			/// effectively be a constant, and should not change during CPU execution.
+			virtual AddressT Size() const = 0;
+
+			virtual u32 Peek(AddressT address) = 0;
+			virtual void Poke(AddressT address, u32 value) = 0;
 		};
 
 		Bus(CPU* cpu);
 		~Bus();
 
+		CPU* GetCPU() { return attachedCpu; }
+
 		/// Attach a device to the bus.
 		///
 		/// Note that once this function is called (and the device is successfully added),
@@ -61,8 +106,9 @@ namespace riscv {
 		/// This function returns true if the device was able to be put on the bus.
 		/// This function returns false in the following error cases:
 		/// 	- [device] is a null pointer
-		///		- The provided base address overlaps a already attached device in some way
+		///		- if [device] is a memory device (and thus reserves address space), adding it would
-		bool AttachDevice(AddressT baseAddress, Device* device);
+		///		  end up shadowing another previously-added device.
+		bool AttachDevice(Device* device);
 
 		/// Clock all clocked devices mapped onto the bus..
 		void Clock();
@@ -76,12 +122,19 @@ namespace riscv {
 		void PokeWord(AddressT address, u32 value);
 
 	   private:
-		lucore::OptionalRef<Device> FindDeviceForAddress(AddressT address) const;
+		Bus::Device* FindDeviceForAddress(AddressT address) const;
+
+		// TODO: The CPU needs not be a separate member or be treated specially, it too can be a Bus::Device now
+		// In fact that would probably be really clean and elegant for calling Step() properly
 
 		CPU* attachedCpu {};
 
-		// TODO: if this ends up being a hotpath replace with ankerl::unordered_dense
+		/// All devices attached to the bus
-		std::unordered_map<AddressT, Device*> mapped_devices;
+		std::vector<Device*> devices;
+
+		// TODO: if these end up being a hotpath replace with ankerl::unordered_dense
+		std::unordered_map<AddressT, MemoryDevice*> mapped_devices;
+		std::unordered_map<AddressT, MmioDevice*> mmio_devices;
 	};
 
 } // namespace riscv

native/projects/riscv/include/riscv/CPU.hpp

@@ -34,11 +34,18 @@ namespace riscv {
 			u32 extraflags;
 		};
 
+		CPU(Bus* bus);
+
 		State& GetState() { return state; }
 
+
+		// TODO: Handlers for CSR read/write
+
+		u32 Step(u32 elapsedMicroseconds, u32 instCount);
+
 	   private:
 		State state;
-		Bus bus;
+		Bus* bus;
 	};
 
 } // namespace riscv

native/projects/riscv/include/riscv/Devices/RamDevice.hpp

@@ -3,15 +3,15 @@
 namespace riscv::devices {
 
 	/// A block of RAM which can be used by the CPU.
-	struct RamDevice : public Bus::Device {
+	struct RamDevice : public Bus::MemoryDevice {
-		RamDevice(AddressT size);
+		RamDevice(AddressT base, AddressT size);
 		virtual ~RamDevice();
 
 		// Implementation of Device interface
 
+		AddressT Base() const override;
 		AddressT Size() const override;
 
-		void Attached(Bus* bus, AddressT base) override;
 
 		u8 PeekByte(AddressT address) override;
 		u16 PeekShort(AddressT address) override;
@@ -25,12 +25,10 @@ namespace riscv::devices {
 		/// helper used for implementing Peek/Poke API
 		template <class T>
 		constexpr usize AddressToIndex(AddressT address) {
-			return ((address - baseAddress) % memorySize) / sizeof(T);
+			return ((address - memoryBase) % memorySize) / sizeof(T);
 		}
 		
-		// remember what we were attached to via "signal"
+		AddressT memoryBase {};
-		Bus* attachedBus {};
-		AddressT baseAddress {};
 
 		u8* memory {};
 		usize memorySize {};

native/projects/riscv/include/riscv/System.hpp

@@ -0,0 +1,37 @@
+#include <riscv/Bus.hpp>
+#include <riscv/Devices/RamDevice.hpp>
+
+namespace riscv {
+
+	// fwd decls
+	struct CPU;
+
+	/// a system.
+	struct System {
+
+		/// Create
+		static System* WithMemory(AddressT ramSize);
+
+		void AddDevice(Bus::MmioDevice* device);
+
+		/// returns false if the cpu broke execution
+		bool Step();
+
+		CPU* GetCPU();
+		Bus* GetBus();
+
+	   private:
+	   	
+		/// How many Cycle() calls will the bus get
+		/// (also decides ipsRate)
+	   	u32 cycleRate;
+
+		/// How many instructions will the CPU execute each step
+		u32 ipsRate;
+
+		CPU* cpu;
+		Bus* bus;
+		devices::RamDevice* ram;
+	}
+
+} // namespace riscv

native/projects/riscv/src/Bus.cpp

@@ -1,88 +1,117 @@
 #include <algorithm>
 #include <riscv/Bus.hpp>
 
+#include "riscv/Types.hpp"
+
 namespace riscv {
 
-	Bus::Bus(CPU* cpu)
+	Bus::Bus(CPU* cpu) : attachedCpu(cpu) {
-		: attachedCpu(cpu) {
-
 	}
 
 	Bus::~Bus() {
 		// Free all devices
-		for(auto& pair : mapped_devices)
+		for(auto device: devices)
-			delete pair.second;
+			delete device;
 	}
 
-	bool Bus::AttachDevice(AddressT baseAddress, Device* device) {
+	bool Bus::AttachDevice(Device* device) {
 		if(!device)
 			return false;
 
+		if(device->IsA<MemoryDevice*>()) {
+			auto* upcasted = device->Upcast<MemoryDevice*>();
+
 			// Refuse to overlap a device at its base address..
-		if(FindDeviceForAddress(baseAddress))
+			if(FindDeviceForAddress(upcasted->Base()))
 				return false;
 			// ... or have the end overlap the start of another device.
-		else if(FindDeviceForAddress(baseAddress + device->Size()))
+			else if(FindDeviceForAddress(upcasted->Base() + upcasted->Size()))
 				return false;
 
-		mapped_devices[baseAddress] = device;
+			mapped_devices[upcasted->Base()] = upcasted;
+		} else if(device->IsA<MmioDevice*>()) {
+			auto* upcasted = device->Upcast<MmioDevice*>();
+
+			// Refuse to overlap a device at its base address..
+			if(FindDeviceForAddress(upcasted->Base()))
+				return false;
+			// ... or have the end overlap the start of another device.
+			else if(FindDeviceForAddress(upcasted->Base() + upcasted->Size()))
+				return false;
+
+			mmio_devices[upcasted->Base()] = upcasted;
+		}
+
+		devices.push_back(device);
+
 		return true;
 	}
 
 	void Bus::Clock() {
-		for(auto& device : mapped_devices) {
+		for(auto device : devices) {
-			if(device.second->Clocked())
+			if(device->Clocked())
-				device.second->Clock();
+				device->Clock();
 		}
 	}
 
 	u8 Bus::PeekByte(AddressT address) {
-		if(auto opt = FindDeviceForAddress(address); opt)
+		if(auto dev = FindDeviceForAddress(address); dev)
-			return opt->PeekByte(address);
+			return dev->Upcast<MemoryDevice*>()->PeekByte(address);
 		return -1;
 	}
 
 	u16 Bus::PeekShort(AddressT address) {
-		if(auto opt = FindDeviceForAddress(address); opt)
+		if(auto dev = FindDeviceForAddress(address); dev)
-			return opt->PeekShort(address);
+			return dev->Upcast<MemoryDevice*>()->PeekShort(address);
 		return -1;
 	}
 
 	u32 Bus::PeekWord(AddressT address) {
-		if(auto opt = FindDeviceForAddress(address); opt)
+		if(auto dev = FindDeviceForAddress(address); dev) {
-			return opt->PeekWord(address);
+			if(dev->IsA<MmioDevice*>()) 
+				return dev->Upcast<MmioDevice*>()->Peek(address);
+			else
+				return dev->Upcast<MemoryDevice*>()->PeekWord(address);
+		}
 		return -1;
 	}
 
 	void Bus::PokeByte(AddressT address, u8 value) {
-		if(auto opt = FindDeviceForAddress(address); opt)
+		if(auto dev = FindDeviceForAddress(address); dev)
-			return opt->PokeByte(address, value);
+			return dev->Upcast<MemoryDevice*>()->PokeByte(address, value);
 	}
 
 	void Bus::PokeShort(AddressT address, u16 value) {
-		if(auto opt = FindDeviceForAddress(address); opt)
+		if(auto dev = FindDeviceForAddress(address); dev)
-			return opt->PokeShort(address, value);
+			return dev->Upcast<MemoryDevice*>()->PokeShort(address, value);
 	}
 
 	void Bus::PokeWord(AddressT address, u32 value) {
-		if(auto opt = FindDeviceForAddress(address); opt)
+		if(auto dev = FindDeviceForAddress(address); dev) {
-			return opt->PokeWord(address, value);
+			if(dev->IsA<MmioDevice*>()) 
+				dev->Upcast<MmioDevice*>()->Poke(address, value);
+			else
+				dev->Upcast<MemoryDevice*>()->PokeWord(address, value);
+		}
 	}
 
-	lucore::OptionalRef<Bus::Device> Bus::FindDeviceForAddress(AddressT address) const {
+	Bus::Device* Bus::FindDeviceForAddress(AddressT address) const {
-		auto it = std::find_if(mapped_devices.begin(), mapped_devices.end(), [&](const auto& pair) {
+		auto try_find_device = [&](auto container, AddressT address) {
+			return std::find_if(container.begin(), container.end(), [&](const auto& pair) {
 				return
 				// We can shorcut region checking if the requested addess matches base address.
 				pair.first == address ||
 				// If it doesn't we really can't, though.
 				(address >= pair.first && address < pair.first + pair.second->Size());
 			});
+		};
 
-		// No device was found at this address
+		if(auto it = try_find_device(mapped_devices, address); it != mapped_devices.end())
-		if(it == mapped_devices.end())
+			return static_cast<Bus::Device*>(it->second);
-			return lucore::NullRef;
+		else if(auto it = try_find_device(mmio_devices, address); it != mmio_devices.end())
-		else
+			return static_cast<Bus::Device*>(it->second);
-			return *it->second;
+
+		return nullptr;
 	}
 
 } // namespace riscv

native/projects/riscv/src/Devices/RamDevice.cpp

@@ -4,7 +4,7 @@
 
 namespace riscv::devices {
 
-	RamDevice::RamDevice(AddressT size) : Bus::Device(), memorySize(size) {
+	RamDevice::RamDevice(AddressT base, AddressT size) : memoryBase(base), memorySize(size) {
 		memory = new u8[size];
 		LUCORE_CHECK(memory, "Could not allocate buffer for memory device with size 0x{:08x}.",
 					 size);
@@ -15,15 +15,12 @@ namespace riscv::devices {
 			delete[] memory;
 	}
 
-	AddressT RamDevice::Size() const {
+	AddressT RamDevice::Base() const {
-		return memorySize;
+		return memoryBase;
 	}
 
-	// Implementation of Device interface
+	AddressT RamDevice::Size() const {
-
+		return memorySize;
-	void RamDevice::Attached(Bus* bus, AddressT base) {
-		attachedBus = bus;
-		baseAddress = base;
 	}
 
 	u8 RamDevice::PeekByte(AddressT address) {

native/projects/riscv_test_harness/CMakeLists.txt

@@ -0,0 +1,7 @@
+add_executable(rvtest
+	main.cpp
+)
+
+target_link_libraries(rvtest PUBLIC
+	riscv::riscv
+)

native/projects/riscv_test_harness/main.cpp

@@ -0,0 +1,32 @@
+#include <riscv/Bus.hpp>
+#include <riscv/CPU.hpp>
+#include <riscv/Devices/RamDevice.hpp>
+
+#include "riscv/Types.hpp"
+
+/// simple 16550 UART implementation
+struct SimpleUartDevice : public riscv::Bus::MmioDevice {
+	constexpr static riscv::AddressT BASE_ADDRESS = 0x10000000;
+
+	riscv::AddressT Base() const override { return BASE_ADDRESS; }
+
+	riscv::AddressT Size() const override { return 5; }
+
+	// TODO: emulate properly
+	u32 Peek(riscv::AddressT address) override {
+		switch(address) {
+			case BASE_ADDRESS:
+				break;
+			case BASE_ADDRESS + 5:
+				break;
+		}
+
+		return 0;
+	}
+
+	void Poke(riscv::AddressT address, u32 value) override {
+		if(address == BASE_ADDRESS) { // write to data buffer
+			printf("%c\n", value);
+		}
+	}
+};