Generic CSR Functionality #151

kimmeljo · 2025-01-03T01:18:11Z

Description & Motivation

Adding a mechanism to generically generate a module to define, read and write control/status registers (CSRs). This functionality includes automatic construction of CSRs based on configuration objects down to the granularity of register fields.

Related Issue(s)

N/A

Testing

Added unit tests (csr_test.dart)

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

New functionality so no breaking changes.

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

A new document describing all functionality will be added.

mkorbel1

i really like this approach, looking good!

mkorbel1 · 2025-01-03T19:04:08Z

lib/src/memory/csr.dart

split into multiple files?

Yeah I like that.

how about even more files? :)

let's try to aim for 1 class per file (except for small, supporting classes)

lib/src/memory/csr.dart

mkorbel1 · 2025-01-03T21:02:35Z

lib/src/memory/memories.dart

@@ -1,5 +1,6 @@
 // Copyright (C) 2023-2024 Intel Corporation


update date year

test/csr_test.dart

ganewto · 2025-01-13T04:56:05Z

I'm late to commenting on this PR.

I didn't see a test for the legalValues restriction. Sometimes it helps to have short burst tests for pieces of functionality to provide a 'smoke test'. It's harder to find in a longer sequence of setup, though easier to amortize that over multiple tests. A strategy could be to setup the CR you want to use at the beginning of a group test and then do a bunch of shorter 'test' functions underneath:

group('CSR big test', () {
//  setup CRs here
... bunch of code
// end setup CRs 
  test('feature access one', () {
  });
  
  test ('feature access two', () {
  });
  ...
 });

ganewto · 2025-01-13T04:57:18Z

Need a doc (csr.md) where you can describe the overall architecture and feature set. Could be just a few short paragraphs, maybe a code example with init and with access and a bit of inspiration on how clean this is.

…in blocks.

…f just on a full register. Adding validation on reset and legal values to ensure that they fit within their allotted bits.

lib/src/memory/csr_config.dart

mkorbel1 · 2025-01-21T19:00:06Z

lib/src/memory/csr_config.dart

+  /// Helper to derive a reset value for the register from its fields.
+  ///
+  /// Only should be used if a reset value isn't explicitly provided.
+  int resetValueFromFields() {


should this API be exposed publicly? since it might not be the "real" reset value if a resetValue for the whole register is provided. you could make it private or even @internal

This is actually used in a encapsulating class (CsrBlock). Is there an annotation that enables the proper protections given this information?

mkorbel1 · 2025-01-21T20:58:30Z

lib/src/memory/csr.dart

how about even more files? :)

let's try to aim for 1 class per file (except for small, supporting classes)

mkorbel1 · 2025-01-21T22:18:52Z

lib/src/memory/csr.dart

+  bool get isBackdoorWritable => config.isBackdoorWritable;
+
+  /// Getter for the field configuration of the CSR
+  List<CsrFieldConfig> get fields => config.fields;


this Csr Logic object is created at the point of time when it is constructed, but the fields property of the config could theoretically be changed by a user after the fact, meaning this fields property could become out of sync from the actual Logics present. This would be silly to do intentionally, but it should be possible to prevent accidentally doing something like that. This ties into that making fields unmodifiable idea from other comments. We should decide how we want these configuration objects to behave: should they be immutable? should they be copied at the time they are consumed?

I see the point. My opinion is that they should be immutable within an encapsulating HW object (i.e., passed during construction, readable to users, but not modifiable). I'm guessing there's nice Dart syntax to do this?

mkorbel1 · 2025-01-21T22:20:43Z

lib/src/memory/csr.dart

+      for (var i = 0; i < elements.length; i++) {
+        // if the given field is reserved or read only
+        // take the current value instead of the new value
+        final chk1 = rsvdIndices.contains(i);


here and for chk2, why not just move the checking condition into the if condition?

I think it was a vestige of code I ended up refactoring. But is that a big deal?

mkorbel1 · 2025-01-21T22:23:08Z

lib/src/memory/csr.dart

adding a comment to track that we need some documentation added to the doc area for how to use these

mkorbel1 · 2025-01-21T22:34:57Z

lib/src/memory/csr_config.dart

+  /// Method to validate the configuration of a single register.
+  ///
+  /// Must check that its fields are mutually valid.
+  void validate() {


what is intended to call these validate functions? should hardware validate upon construction? is the user expected to call it before using it?

Hardware calls all of these functions appropriately and automatically upon construction/build. But I decided it's perfectly valid to also allow a user to call the validate function independently on the config object if they want.

mkorbel1 · 2025-01-21T22:36:48Z

lib/src/memory/csr_config.dart

+  /// register validation is called separately (i.e., in Csr HW construction).
+  void validate() {
+    // at least 1 register
+    if (registers.isEmpty) {


does this have to be illegal? is an empty block so bad?

I guess it's not terrible but it doesn't really make sense to me....

mkorbel1 · 2025-01-21T22:38:02Z

lib/src/memory/csr_config.dart

+  /// based on the maximum block base address. Note that we independently
+  /// validate the block offset width relative to the base addresses
+  /// so we can trust the simpler analysis here.
+  int minAddrBits() {


should these be private helper functions rather than public API?

These are currently called outside of the class (during HW construction for validation) but by an encapsulating class. If there's a better annotation, I'm open but also I kind of like having them be public (why not allow users to call them?).

kimmeljo added 3 commits December 20, 2024 17:04

WIP: starting generic CSR implementation. Still some work to do...

151da01

WIP: progress on csrs.

1ad1347

Progress on CSR stuff. Simple tests now exist too.

95f7ceb

mkorbel1 reviewed Jan 3, 2025

View reviewed changes

kimmeljo added 3 commits January 7, 2025 15:07

Addressing various feedback items.

b976055

Small tweaks.

bfc192b

Adding backdoor read and write functionality.

1add2c5

kimmeljo added 7 commits January 13, 2025 15:24

More testing and bug fix with backdoor ports.

f4dacb5

Adding frontdoor and backdoor visibility as a config option.

6a41c62

Adding dynamic behavior for frontdoor read and writability.

a3285c8

Adding validation to all Csr config objects.

bba27cd

Adding validation to frontdoor interface widths relative to configs.

6351972

Adding support for heterogeneous register widths both across and with…

b89cdcd

…in blocks.

Adding the ability to set reset values on individual fields instead o…

6a98c5e

…f just on a full register. Adding validation on reset and legal values to ensure that they fit within their allotted bits.

mkorbel1 reviewed Jan 21, 2025

View reviewed changes

Addressing various feedback items and fixing lint warnings.

f29b46f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generic CSR Functionality #151

Generic CSR Functionality #151

kimmeljo commented Jan 3, 2025

mkorbel1 left a comment

mkorbel1 Jan 3, 2025

kimmeljo Jan 3, 2025

mkorbel1 Jan 21, 2025

mkorbel1 Jan 3, 2025

ganewto commented Jan 13, 2025

ganewto commented Jan 13, 2025

mkorbel1 Jan 21, 2025

kimmeljo Jan 21, 2025

mkorbel1 Jan 21, 2025

mkorbel1 Jan 21, 2025

kimmeljo Jan 21, 2025

mkorbel1 Jan 21, 2025

kimmeljo Jan 24, 2025

mkorbel1 Jan 21, 2025

mkorbel1 Jan 21, 2025

kimmeljo Jan 21, 2025

mkorbel1 Jan 21, 2025

kimmeljo Jan 21, 2025

mkorbel1 Jan 21, 2025

kimmeljo Jan 21, 2025

Generic CSR Functionality #151

Are you sure you want to change the base?

Generic CSR Functionality #151

Conversation

kimmeljo commented Jan 3, 2025

Description & Motivation

Related Issue(s)

Testing

Backwards-compatibility

Documentation

mkorbel1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ganewto commented Jan 13, 2025

ganewto commented Jan 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment