search

pulp-platform / redmule

Other
32 stars 12 forks source link

Integrate ECC-extended HCI. #27

Closed LuigiGhionda closed 1 month ago

LuigiGhionda commented 3 months ago

This PR extends HCI modules of the streamer with ECC and add memory-mapped registers to collect statistics on occurred fault

LuigiGhionda commented 3 months ago

I created a separate branch from the "hci-v2.1" branch and set it as the target of the PR because the "astral" branch has diverged. I hope this is ok

yvantor commented 3 months ago

I created a separate branch from the "hci-v2.1" branch and set it as the target of the PR because the "astral" branch has diverged. I hope this is ok

Yes, it is more than fine!

LuigiGhionda commented 3 months ago

Approved. If you did not do it yet, I would suggest to run the regression script you find under scripts for a general sanity check. Also write to Maurus to compare because he recently found some bugs in the sw test. Let me know if the two steps above are complete and then we can merge!

Yes, I ran the regression script, including Maurus's modifications. It was only necessary to slightly increase the timeout time for each test from 100s to 120s, which should be related to the overhead of reading the error-counting registers.

Lynx005F commented 2 months ago

Can you also update the scripts/non-regression_test.sh do reflect your changes? E.g. run it with ECC enabled and disabled, update timeout etc.

LuigiGhionda commented 2 months ago

Can you also update the scripts/non-regression_test.sh do reflect your changes? E.g. run it with ECC enabled and disabled, update timeout etc.

Done. Thank you for the reminder

Lynx005F commented 2 months ago

Based of this branch I made a version that only does the ECC Encode / Decode at the very outside: https://github.com/Lynx005F/redmule/commits/itemm/hci-v2.1-ecc-outside/

Thanks to your regression script I could test it works right away ;), I think I will most likely rebase the other redundancy-related changes onto it first and then see if I can find ways to protect the streamer control and W path on top of it.

Lynx005F commented 2 months ago

Ah I see you used the same approach now as well, so I can rebase the other redundancy stuff on top of it. Neat!

yvantor commented 1 month ago

@LuigiGhionda is this also the version currently used in PULP cluster?

yvantor commented 1 month ago

@LuigiGhionda is this also the version currently used in PULP cluster?

I checked. Merging!

LuigiGhionda commented 1 month ago

@LuigiGhionda is this also the version currently used in PULP cluster?

Yes, exactly (late confirmation)