[SW] change periodic tasks to fixed cycle counts #85

ANurmi · 2025-02-19T10:41:27Z

Plus update rt-ibex reference.
Need to clean up before merging, but creating this already as the produced results are likely final.

hegza · 2025-02-19T11:57:10Z

Please run cargo fmt from examples/periodic_tasks directory.

Consider also turning on auto-formatting for this workspace by adding to .vscode/settings.json: "editor.formatOnSave": true. This should reduce the amount of back-and-forth changes with subsequent git commits as every commit produces standard format. Be mindful that this VSCode option is a little dangerous for a project that does not use a standard formatting policy as it can clutter your git, so consider making it so that it only applies to this software workspace.

hegza · 2025-02-19T12:13:56Z

Merge will not be clean due to updated Timer API. I propose rebasing onto latest main; we can look at it together if you haven't done it before.

EDIT: I do recommend to get this working functionally perfect, before considering the rebase -- that will be the last thing to do before merge.

hegza

I got the idea. ~~Seems functionally correct which is great.~~ I propose you implement changes to improve readability as requested and submit for re-review. Then let me know and I can do the rebase, or we can do it together if you'd like to learn about that.

Bender.lock

examples/periodic_tasks/src/main.rs

hegza · 2025-02-19T11:59:27Z

examples/periodic_tasks/src/main.rs

@@ -10,7 +10,7 @@ use bsp::{
    embedded_io::Write,
    mtimer::{self, MTimer},
    nested_interrupt,
-    riscv::{self, asm::wfi},
+    riscv::{self, asm::{self, nop, wfi}, read_csr, register::{cycle, mcounteren}},


Some of these are unused. The linter should underline them in yellow. Please remove unused imports.

examples/periodic_tasks/src/main.rs

hegza · 2025-02-19T12:36:27Z

examples/periodic_tasks/src/main.rs

+            // TODO: figure out bsp access to CSRs
+            let mut cycle_lo: u32;
+            let mut cycle_hi: u32;
+            let mut retired_lo: u32;
+            let mut retired_hi: u32;
+            core::arch::asm!("csrr {0}, 0xB00", out(reg) cycle_lo);
+            core::arch::asm!("csrr {0}, 0xB80", out(reg) cycle_hi);
+            core::arch::asm!("csrr {0}, 0xB02", out(reg) retired_lo);
+            core::arch::asm!("csrr {0}, 0xB82", out(reg) retired_hi);
+            sprintln!("cycles: {}{}", cycle_hi,   cycle_lo );
+            sprintln!("instrs: {}{}", retired_hi, retired_lo );


Here you go:

// Read top & bottom halves into one 64-bit binding: let cycle = riscv::register::cycle::read64(); // Read halves separately let cycle_lo = riscv::register::cycle::read(); let cycle_hi = riscv::register::cycleh::read(); // For "retired", same as above but `minstret` let minstret = riscv::register::minstret::read64();

With latest main, same can be achieved slightly more idiomatically via bsp::register::cycle::read64, etc., but you shouldn't care about that. Just use the above code.

The example code (both versions) cause an illegal instruction, need to debug.

mcycle, not cycle (which is supervisor variant)

examples/periodic_tasks/src/main.rs

hegza

To assert that the test doesn't break by changes to periph_clk_div, I propose you adjust the start of main as follows:

#[entry]
fn main() -> ! {
    // Assert that periph clk div is as configured
    write_u32(CFG_BASE + PERIPH_CLK_DIV_OFS, PERIPH_CLK_DIV);

    let mut serial = ApbUart::init(CPU_FREQ, 115_200);
    sprintln!("[periodic_tasks (PCS={:?})]", cfg!(feature = "pcs"));
    sprintln!(
        "PERIPH_CLK_DIV: {}",
        read_u32(CFG_BASE + PERIPH_CLK_DIV_OFS)
    );

Also change PERIPH_CLK_DIV to u32 (which is it's native representation) then cast as necessary.

hegza

Task duration saturates to zero due to division. Precision must be increased from us to ns.

Fixes the problem with value saturation.

hegza · 2025-02-19T13:54:57Z

Task duration saturates to zero due to division. Precision must be increased from us to ns.

Fixed by the latest commit.

hegza

Seems functionally correct :D maybe for real this time. There's at least the periph clk div issue which is critical, rest is style & re-enabling code, I think. I can adopt the branch after this round.

examples/periodic_tasks/src/main.rs

CHANGELOG.md

hegza · 2025-02-20T15:01:10Z

examples/periodic_tasks/src/main.rs

+            // TODO: figure out bsp access to CSRs
+            let mut cycle_lo: u32;
+            let mut cycle_hi: u32;
+            let mut retired_lo: u32;
+            let mut retired_hi: u32;
+            core::arch::asm!("csrr {0}, 0xB00", out(reg) cycle_lo);
+            core::arch::asm!("csrr {0}, 0xB80", out(reg) cycle_hi);
+            core::arch::asm!("csrr {0}, 0xB02", out(reg) retired_lo);
+            core::arch::asm!("csrr {0}, 0xB82", out(reg) retired_hi);
+            sprintln!("cycles: {}{}", cycle_hi,   cycle_lo );
+            sprintln!("instrs: {}{}", retired_hi, retired_lo );


mcycle, not cycle (which is supervisor variant)

hegza · 2025-02-20T15:04:40Z

examples/periodic_tasks/src/main.rs

+            let total_in_task0 = TASK0.duration_ns * TASK0_COUNT as u32;
+            let total_in_task1 = TASK1.duration_ns * TASK1_COUNT as u32;
+            let total_in_task2 = TASK2.duration_ns * TASK2_COUNT as u32;
+            let total_in_task3 = TASK3.duration_ns * TASK3_COUNT as u32;


tbh best to rename these to total_ns_in_task0, etc.

hegza · 2025-02-20T15:05:53Z

examples/periodic_tasks/src/main.rs

                total_in_task0,
                total_in_task1,
                total_in_task2,
                total_in_task3,
                total_in_task0 + total_in_task1 + total_in_task2 + total_in_task3,
            );

+            /* TODO: reintroduce after mtimer div fixed


Yep. I think for now it should be OK to just change the const PERIPH_CLK_DIV: u32 = 2; to 1 from atalanta_bsp/src/mtimer.rs and atalanta_bsp/src/timer_group.rs. Then re-introduce these.

ANurmi · 2025-02-21T11:17:36Z

Should be all good now, @hegza please merge.

This makes them take the expected amount of time.

ANurmi and others added 14 commits February 13, 2025 15:50

Rust env test

53300a4

Add WIP

8bb3498

Add WIP for debug

9447c7e

WIP, results semi-stable

e4cdcda

ccs -10%, ins -13%

540f77d

Clear counters, reduction ~13%

90e4bb6

Balance sleep cycles

d5d6c75

cc -18%, ins -27%

43cdabc

Add debug WIP

2a74b27

hardware stacking results for the intensive test

a7d7fd8

Add comment

046f9cb

Update rt-ibex

9355ab3

Correct rt-ibex hash

1588e4b

Update rt-ibex

d6bfa6f

ANurmi requested a review from hegza February 19, 2025 10:41

hegza requested changes Feb 19, 2025

View reviewed changes

hegza reviewed Feb 19, 2025

View reviewed changes

hegza requested changes Feb 19, 2025

View reviewed changes

hegza added 2 commits February 19, 2025 15:08

chore: LEs

3c08d43

Increase timer input value precision from us to ns

fdc7a13

Fixes the problem with value saturation.

ANurmi added 4 commits February 20, 2025 10:53

Merge branch 'main' into sw/fixed-tasks

e322c51

Remove phase from task struct

4251af6

cargo fmt

d3bf329

Change task percentages to promille

3f5ccff

ANurmi marked this pull request as ready for review February 20, 2025 13:44

hegza requested changes Feb 20, 2025

View reviewed changes

Allow off-by-one in test validation code

cae2360

Update results, fix rt-ibex reference

2a59531

ANurmi and others added 5 commits February 21, 2025 13:34

Fix bad rt-core config

da3d0e8

chore: format markdown

358b210

chore(periodic_tasks): update lock-file

7a75a58

chore: rm redundant cast

ec68a42

bench(periodic_tasks): assert both task count bounds

3c084a7

hegza force-pushed the sw/fixed-tasks branch from 6b24aa0 to 3c084a7 Compare February 21, 2025 12:08

hegza added 2 commits February 21, 2025 15:21

fix(periodic_tasks): use repeat macro for nop loops

d77f3af

This makes them take the expected amount of time.

doc: add measurement advisory

388cd26

hegza approved these changes Feb 21, 2025

View reviewed changes

hegza merged commit 8aab319 into main Feb 21, 2025
1 check passed

hegza deleted the sw/fixed-tasks branch February 21, 2025 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SW] change periodic tasks to fixed cycle counts #85

[SW] change periodic tasks to fixed cycle counts #85

ANurmi commented Feb 19, 2025

hegza commented Feb 19, 2025

hegza commented Feb 19, 2025 •

edited

Loading

hegza left a comment •

edited

Loading

hegza Feb 19, 2025

hegza Feb 19, 2025

hegza Feb 19, 2025

ANurmi Feb 20, 2025

hegza Feb 20, 2025

hegza left a comment

hegza left a comment

hegza commented Feb 19, 2025

hegza left a comment

hegza Feb 20, 2025

hegza Feb 20, 2025

hegza Feb 20, 2025

ANurmi commented Feb 21, 2025

[SW] change periodic tasks to fixed cycle counts #85

[SW] change periodic tasks to fixed cycle counts #85

Conversation

ANurmi commented Feb 19, 2025

hegza commented Feb 19, 2025

hegza commented Feb 19, 2025 • edited Loading

hegza left a comment • edited Loading

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

hegza left a comment

Choose a reason for hiding this comment

hegza left a comment

Choose a reason for hiding this comment

hegza commented Feb 19, 2025

hegza 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

ANurmi commented Feb 21, 2025

hegza commented Feb 19, 2025 •

edited

Loading

hegza left a comment •

edited

Loading