Adding I2C module support

[Froglodyte]

Overview:

Adds I2C module to the Olympia RISC-V simulator. Ive implemented it as memory mapped peripheral within the MSS, so the CPU can perform load/store operations to specific I2C addresses.

Key Changes:

1. New I2C Unit (mss/I2C.hpp, mss/I2C.cpp):

• Created olympia_mss::I2C (inherits from sparta::Unit of course)
• Configurable i2c_latency (default is 10 cycles) and address range.
• Handles incoming memory requests and sends acknowledgments back to the BIU.

2. BIU Routing Logic (mss/BIU.hpp, mss/BIU.cpp):

• Added i2c_addr and i2c_size parameters.
• Actual Routing logic: if a request address falls within [i2c_addr, i2c_addr + i2c_size), it is routed to the I2C port. Otherwise, itll go to DRAM.

3. Testing:

• Verification Trace: Added traces/i2c_test.json containing load instructions targeting the I2C address space.
• Validation: Ran simulations confirming that requests to the configured I2C address (0x48000000 in testing) are correctly routed to the I2C unit, processed with the specified latency, and acknowledged.

partially addresses the need for peripheral simulation support in Olympia (Issue #56) .

[klingaard]

This is awesome -- loving the addition.

Since you're the trailblazer here, how about you start this off generically? Here's what I propose...

Instead of the BIU knowing anything about I2C, allow the BIU to be "programmed" to set up special regions of memory. Start with the new BIU parameters.

Change these

PARAMETER(uint64_t, i2c_addr, 0x40000000, "I2C start address")
PARAMETER(uint64_t, i2c_size, 0x1000, "I2C address space size")

to this (untested/uncompiled but should work with some C++ magic):

struct MappedDevice
{
     uint64_t addr = 0;
     uint32_t size = 0;
     std::string device_name;
};

PARAMETER(std::vector<MappedDevice>, mapped_devices, {}, R"(Vector of Mapped Devices in simulation.  

Example: 
    top.*.biu.mapped_devices "[[0x40000000, 0x1000, \"i2c\"]]"

)")

In the BIU's constructor, walk the mapped_devices parameter and for each mapped device listed

1. Make sure there are no overlaps with previous mappings
2. Dynamically create in/out ports based on the device_name
3. The acknowledge methods should not care where the ack came from, so those should be generic.

Finally, creating new units like I2C should be parameterized, but this is not really set up well in Olympia. I know how to do it, so when you get to that point, I can help.

If you get stuck with the parameter, let me know. We've done this before and it's a little tricky (HINT: put operator<< and operator>> in the std namespace -- you'll see why 😉 )

[Froglodyte]

@klingaard that sounds great, but I have a few questions before I work on those improvements-
first, is mss/BIU.hpp an acceptable location for the MappedDevice struct, or should i move that into its own separate .hpp file?
and second, how should address overlaps be handled? should the simulation just throw a sparta::SpartaException and abort?

[klingaard]

@klingaard that sounds great, but I have a few questions before I work on those improvements- first, is mss/BIU.hpp an acceptable location for the MappedDevice struct, or should i move that into its own separate .hpp file? and second, how should address overlaps be handled? should the simulation just throw a sparta::SpartaException and abort?

Yes, for now, the BIU is an acceptable place for the map. Eventually we'll move this to some kind of coherency manager, but that requires some contemplating.

If you detect an overlap, throw a sparta::SpartaException with as much detail as possible.

[Froglodyte]

@klingaard Hi, I've got the primary logic for configurable memory regions for devices down in this new commit. Now its time to parameterise creating new units. I'm guessing this takes place in the yaml config files, but I'll need more details to make sure I get it right.

To summarize the new logic:

1. I added the MappedDevices struct, as you mentioned. It uses istream and ostream to parse the configuration string more easily. I feel like it may be overkill but it also seems like the most efficient solution?
2. It handles dynamic port allocation and memory overlap checks in the same loop in BIU.cpp. As for dynamic routing, when a request arrives, instead of checking if (addr == I2C_ADDR), it scans the list of configured devices. If the address falls within a devices range, the request is sent to that specific device's dynamically created port, otherwise it defaults to the MSS.
3. All devices behave similarly (receive request -> do work -> send ack), so the BIU doesnt need unique code for each. BIU::handleDeviceAck_ handles the signal from any attached device.
4. I created a block in CoreTopologySimple::bindTree to check for mapped devices. It connects all the BIU dynamic ports to the actual device ports during startup.

[klingaard]

An error message here would be good, like "Malformed parameter for mapped device: . Expected: "

[klingaard]

This is looking good. I'd like an engineer on my team to take a peek at this. She might have a use for it and she can give you some feedback.

[Froglodyte]

@klingaard I've made the changes and also used regex to parse the custom config string instead (seemed more optimal). All thats left now is parameterising the creation of new units.

[Froglodyte]

I've decoupled the peripheral component creator system and the BIU. It's making everything more organised since future plans will make the system more complex. I plan on adding Master/Slave device configs, read/write restrictions, and allowing a component to be mapped to a range of memory addresses:

Summary of Each File:

1. mss/MappedDevice.*: Moved the MappedDevice struct and its logic into a separate file allowing the system to define memory maps without relying on the BIU.
2. mss/DeviceBase.hpp: A standardised base class for all peripherals. It predefines the SyncInPort (for requests) and SyncOutPort (for acks) that the BIU expects.
3. mss/DummyDevice.*: Implemented a mock device. It inherits from DeviceBase and provides basic request handling and logging, allowing for immediate testing of memory maps.
4. arches/peripherals/test_devices.yaml: Created a config file for peripheral address mapping (I2C, GPIO, UART).

Summary of Logic Changes:

1. Dynamic Peripheral Creation: Modified the BIU and core/CPUTopology.cpp to support dynamic instantiation of devices. The BIU can now request the creation of peripherals based on whatever config the user gives.
2. Physical Address Bridge: Updated core/Inst.hpp to allow Load/Store instructions to pass their target address directly to the BIU as a physical address.

[lhastings-mips]

Thanks for making this generic. Looks like a good start to being able to add multiple device endpoints.

[lhastings-mips]

Would it be useful to call this start_addr and then also have a uint64_t stop_addr? The stop address could be computed once and set when the MappedDevice is created, then it can be used in BIU::handleBIUReq_() when checking if a given address falls within the range of a device, instead of adding the size to the start address for every request. stop_addr could also be used when checking for overlaps with previous memory regions in the BIU constructor.

[Froglodyte]

Yes this sounds like a much cleaner implementation. I'll make sure to add it. I have exams coming up so I'll only be able to begin work on these changes after they end.

[lhastings-mips]

Since every device will need to handle a request, perhaps move this to DeviceBase class as a virtual function, to be overridden by each derived class?

[lhastings-mips]

To make sure I understand correctly, is the acknowledgement here just a "Yes, I got your request" type of ack? Or will it be a "I got your request, I spent time processing your request (assuming a parameter to specify latency will be added?), and now that I'm done, I'll send back an ack"?

[Froglodyte]

It's the second one. If it was the first then the BIU would become free again immediately, which could conflict with the BIU's one outstanding request policy. I have parameters for latency in the works right now, and I'll make the commit asap

[lhastings-mips]

Might be worth adding a check that the request can fit entirely within the memory region (as opposed to straddling a boundary). In other words, what if addr is 1 less than device.addr + device.size, but the size of the request is 64 bits? The address falls in the memory region, but the whole access does not. This is probably either a software error or an incorrect programming of the memory regions. In either case, would be good to catch it.

[lhastings-mips]

I'm not as familiar with the Olympia BIU, but does it allow outstanding requests to multiple devices at a time? Or to MSS and a device? If so, it seems possible to receive multiple acks in a single cycle (even if the requests are sent in separate cycles, if the device/mss latencies are different, there could be collisions on when the acks arrive). If multiple acks can be received in a cycle, then you can't guarantee that the request at the front of biu_req_queue_ is the request for this ack. Some sort of check would need to be done.

If the BIU does not allow multiple outstanding requests, then the question is: Should it? Is it reasonable to assume that all requests must be serialized, even if they are to different endpoints?

[Froglodyte]

No, the BIU doesn't allow multiple outstanding requests. Whenever a request is sent, biu_busy_ is set to true, and no new requests can be made until an ack is received. As of right now concurrency doesnt seem to be supported by Olympia.

As for whether serialization is reasonable, I think it's fine for simpler, smaller models, but can be very limiting if you want to model more complex SoCs. Upgrading the BIU to allow concurrency sounds like a pretty difficult task, and not something I can do alone, especially since that design was put in place by the maintainers themselves. What I could do as a workaround here is giving busy flags to each device.

[lhastings-mips]

Agreed that allowing multiple outstanding requests to the BIU is outside the scope of this PR. It was more of a philosophical question. 😊
Given that, this code should be fine. I just wanted to make sure you could always guarantee that what was at the head of the queue is what you are receiving the ack for.

[lhastings-mips]

const

[Froglodyte]

please elaborate. Is const not preferred in the code?

[lhastings-mips]

const is preferred. In this case, device_unit_path should be marked const. Sorry for not being more clear.

[lhastings-mips]

const

[lhastings-mips]

const

[lhastings-mips]

const

[lhastings-mips]

const

[lhastings-mips]

Something we've done before to make the parsing of parameters like this a bit easier is:

PARAMETER(std::vector<std::vector<std::string>>, mapped_devices, {},
                     "List of Mapped Devices in simulation. For each device, "
                      "specifies memory start address, size of memory region "
                      "and name of device. Example: "
                      "[[\"0x40000000\", \"0x1000\", \"i2c\"],"
                      "[\"0x40001000\", \"0x2000\", \"uart\"]]")

The format is still JSON-like, but since it is a vector of vectors, there is no need to search for and consume "[" and "]". Instead, you can iterate through the parameter, something like:

const auto & mapped_devices_param =
    p->mapped_devices.getValueAs<std::vector<std::vector<std::string>>>();
for (const auto & device : mapped_devices_param)
{
    sparta_assert(device.size() == 3, "Mapped device must have 3 elements");
    // assumes the address and size are in hex format, but you could check
    // that the string starts with "0x" and change the base if it doesn't
    const sparta::memory::addr_t start_addr = std::stoul(device[0], nullptr, 16);
    const uint64_t size = std::stoul(device[1], nullptr, 16);
    const std::string device_name = device[2];
    .....
}
                 

[Froglodyte]

Hi @lhastings-mips @klingaard I've found the time to work on this issue again. Before moving forward, I think it would be wise to make the plan I have in mind clearer. I hope there are no conflicts between this and the Execution Driven version of Olympia

1. Overall System Structure

A. BIU acts like a Router-

It'll maintain the:

• Address Map: A list of MappedDevice descriptors.
• Dynamic Port Registry: A map of SyncPorts created at runtime based on the configuration.
• Concurrency Manager: A set of per-device busy flags that allow multiple devices to process requests in parallel without head-of-line blocking.

B. DeviceBase-

Every external component must inherit from the DeviceBase class. It'll handle automated registration of SyncInPort (requests) and SyncOutPort (acks), provide a uniform API (pure virtual handleRequest(const MemoryAccessInfoPtr &req) method that the peripheral must implement.), and builtin timing.

C. MappedDevice-

A metadata structure that defines:

• start_addr: The base address of the peripheral.
• stop_addr: The exclusive limit of the region (pre-computed for $O(1)$ lookup performance).
• device_name: Used to bind the region to a specific hardware instance.

2. Dynamic Instantiation Workflow

The system builds the SoC dynamically.

A. Configuration (YAML): The user provides a list of mapped devices in the architecture YAML:

```yaml
top.cpu.core0.biu.params.mapped_devices: '[[0x40000000, 0x1000, "timer"], [0x40001000, 0x1000, "uart"]]'
``

B. BIU Construction:

*   The BIU parses the string into `MappedDevice` segments.
*   It creates unique `SyncPorts` for each unique `device_name`.
*   It uses `CPUFactories` to instantiate the actual C++ device objects.

C. Topology Binding:

CPUTopology automatically detects the new device nodes and wires their ports to the corresponding BIU ports using the standardized names provided by DeviceBase.

3. Concurrency & Other Cool Stuff-

• Non-Blocking Issue: The BIU can issue a request to Device B even if Device A is currently busy.
• Ordered Acknowledgments: Because the BIU allows only one outstanding request per physical device, incoming acknowledgments are matched to the oldest request in the queue for that specific device name.
• Backpressure: Per-device busy flags prevent the BIU from overwhelming a peripheral that is still processing.

4. How to Add a New External Component

1. Create the Class:

   class MyTimer : public olympia_mss::DeviceBase {
       void handleRequest(const MemoryAccessInfoPtr &req) override {
           // logic and other boring stuff
           out_ack_sync_.send(true, latency_); // Complete with latency
       }
   };

2. Register Factory: Add a sparta::ResourceFactory for your device in CPUFactories.hpp.
3. Map in YAML: Add the address range and your device name to the mapped_devices parameter.

[klingaard]

These are great ideas! Some tweaks worth considering:

Regarding the BIU as a router...

Instead of the BIU being the router, consider making a coherency manager instead. Specifically, create a sparta::Unit that is called CoherencyManager or CM if you prefer. The BIU connects to this unit via SyncPorts and the ratio is programmed at the core_extensions level.

From there you can create a CM factory that iterates over the mapped components and creates either dummy components or looks for specific named factories that the CM can use to create the component. Other information would include:

• Clock ratios from CM to component
• Expected latency
• Number of outstanding transactions the unit can take
• etc....

Each component found in the map is added as a child of the CM.

[klingaard]

@Froglodyte -- what is the status of your work on this PR? Lots of great ideas that I'd like to see this to fruition!
If you cannot continue working on this, let me know. I think we can take it over (and make sure you are co-author).

[Froglodyte]

@klingaard I started some work on the new plan last month, but college work has become a bit overwhelming and I'll be too busy to do anything until June. I think it's best if you guys take over the issue from here. Thank you for the help so far.

[klingaard]

Ok, understood. Do you have any code that has not been pushed yet? I can take your branch from your fork and continue the effort. You will be mentioned as the co-author when the PR is posted. Thanks for starting this effort!

[Froglodyte]

I have nothing important that hasnt been pushed yet.